Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve valgrind client request support to include more targets #1989

Open
1 of 11 tasks
andrewrk opened this issue Feb 19, 2019 · 1 comment
Open
1 of 11 tasks

improve valgrind client request support to include more targets #1989

andrewrk opened this issue Feb 19, 2019 · 1 comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Milestone

Comments

@andrewrk
Copy link
Member

In debug builds, Zig memsets undefined memory to 0xaa, and then inserts a few instructions which provide metadata to Valgrind (or other tools that support this metadata, but I think it's only Valgrind currently). This makes it so that valgrind can detect uninitialized values:

pub fn main() u8 {
    var x: u32 = undefined;
    if (x == 1234) {
        return 1;
    } else {
        return 0;
    }
}
==25307== Conditional jump or move depends on uninitialised value(s)
==25307==    at 0x223C64: main (test2.zig:3)

I did a proof of concept of this with x86_64 (commit coming shortly). However Valgrind has support for more targets:

  • i386 linux, darwin, mingw, solaris
  • i386 windows
  • x86_64 linux, darwin, mingw, solaris
  • ppc32 linux
  • ppc64be linux
  • ppc64le linux
  • arm linux
  • arm64 linux
  • s390x linux
  • mips32 linux
  • mips64 linux
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. stage1 The process of building from source via WebAssembly and the C backend. labels Feb 19, 2019
@andrewrk andrewrk added this to the 1.0.0 milestone Feb 19, 2019
andrewrk added a commit that referenced this issue Feb 19, 2019
with this change, when you assign undefined, zig emits a few
assembly instructions to tell valgrind that the memory is undefined

it's on by default for debug builds, and disabled otherwise. only
support for linux, darwin, solaris, mingw on x86_64 is currently
implemented.

--disable-valgrind turns it off even in debug mode.
--enable-valgrind turns it on even in release modes.

It's always disabled for compiler_rt.a and builtin.a.

Adds `@import("builtin").valgrind_support` which lets code know
at comptime whether valgrind client requests are enabled.

See #1989
@andrewrk
Copy link
Member Author

I went ahead and turned this on by default for debug builds. Here's some timing tests I did:

behavior tests:
  no valgrind integration
  binary size: 1.2M
                  Name       Start         End    Duration     Percent
            Initialize      0.0000      0.0016      0.0016      0.0006
     Semantic Analysis      0.0016      0.6342      0.6326      0.2491
       Code Generation      0.6342      0.7451      0.1109      0.0436
      LLVM Emit Output      0.7451      2.4800      1.7349      0.6831
    Build Dependencies      2.4800      2.5077      0.0278      0.0109
             LLVM Link      2.5077      2.5398      0.0321      0.0126
                 Total      0.0000      2.5398      2.5398      1.0000
  
  
  valgrind integration
  binary size: 1.2M
                  Name       Start         End    Duration     Percent
            Initialize      0.0000      0.0016      0.0016      0.0006
     Semantic Analysis      0.0016      0.6332      0.6316      0.2440
       Code Generation      0.6332      0.7414      0.1082      0.0418
      LLVM Emit Output      0.7414      2.5310      1.7896      0.6915
    Build Dependencies      2.5310      2.5570      0.0260      0.0101
             LLVM Link      2.5570      2.5879      0.0309      0.0120
                 Total      0.0000      2.5879      2.5879      1.0000




std lib tests:
no valgrind integration
binary size: 9.8M
                  Name       Start         End    Duration     Percent
            Initialize      0.0000      0.0068      0.0068      0.0003
     Semantic Analysis      0.0068      3.0785      3.0716      0.1317
       Code Generation      3.0785      4.3512      1.2727      0.0546
      LLVM Emit Output      4.3512     22.7576     18.4064      0.7895
    Build Dependencies     22.7576     23.0984      0.3408      0.0146
             LLVM Link     23.0984     23.3148      0.2163      0.0093
                 Total      0.0000     23.3148     23.3148      1.0000


valgrind integration
binary size: 11M
                  Name       Start         End    Duration     Percent
            Initialize      0.0000      0.0036      0.0036      0.0001
     Semantic Analysis      0.0036      3.0812      3.0777      0.1264
       Code Generation      3.0812      4.3706      1.2894      0.0530
      LLVM Emit Output      4.3706     23.5837     19.2132      0.7893
    Build Dependencies     23.5837     24.1339      0.5501      0.0226
             LLVM Link     24.1339     24.3428      0.2089      0.0086
                 Total      0.0000     24.3428     24.3428      1.0000

You can see that in practice for debug builds, it makes compile time 4% slower and makes the binary size 11% bigger. Release builds are unchanged, unless you use --enable-valgrind.

@Vexu Vexu removed the stage1 The process of building from source via WebAssembly and the C backend. label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Projects
None yet
Development

No branches or pull requests

2 participants