Rewriter fails to emit used attribute for static fns when using a macro

[randomPoison]

The source rewriter does not emit the __attribute__((used)) attribute for static functions if the function declaration begins with a macro. This attribute is required for static address-taken functions, otherwise we get linker errors like the ones described below.

In zlib this issue occurred because the static keyword was inside of a preprocessor define:

#define local static
local void function() {}

If we hit this case, the rewriter does emit a warning "non-file loc for function", but this warning is not descriptive enough to be actionable and is easy to miss because the rewriter doesn't fail if this is hit. We should make the message more descriptive so that users know they need to add the used attribute manually.

---

Original description

In zlib we have some static address-taken functions that the rewriter has rewritten and generated call gates for, e.g. deflate_fast. These both get compiled into libz.so, which is then used as a dependency when linking the final binary. However, we get linker errors when we try to link the final binary:

/usr/bin/ld: libz.so.1.3.1.1-motley: undefined reference to `deflate_fast'

If I run nm libz.so | rg deflate_ I get:

0000000000018480 R deflate_copyright
                 U deflate_fast
                 U deflate_slow
                 U deflate_stored
0000000000005305 t __ia2_deflate_fast
00000000000053bd t __ia2_deflate_slow
0000000000005475 t __ia2_deflate_stored

So deflate_fast is showing up as an undefined reference in libz.so, even though deflate_fast is defined within that shared object.

Removing the static modifier resolves the issue, but isn't an ideal solution here. We currently are deliberately generating the callgate assembly within the same file as the function specifically so that we can directly reference the function. But for some reason this doesn't seem to be working for zlib.

[ayrtonm]

Do you have a link to the case that caused the error? In general we handle taking the address of static functions and I'm not sure why the wrappers you got for these static functions were in the call gates .so (they should be in the .c with the static function).

[randomPoison]

@rinon was able to find out what was happening here, so I've updated the issue name and description to better describe what's happening. The relevant zlib source code is the deflate_fast function in the rewritten sources. Note that the used attribute was not inserted by the rewriter.

[randomPoison]

We should probably also document this limitation in the usage docs.

[ayrtonm]

This can probably be fixed in the rewriter

[rinon]

I looked at fixing it, I don't think it's worthwhile to fix. It involves deep in clang's macro expansion code, and we can't fix it in the general sense without rewriting macro contents, which is a losing game. I'd prefer we just fix the warning.

[ayrtonm]

Totally agree that rewriting macro contents is a losing game, but this replacement is just prepending __attribute__((used)). I think duplicate attributes are allowed so if we can get the location of the macro invocation-site we might be able to just add __attribute__((used)) again? Not sure if that would break other corner cases though.

[ayrtonm]

oh actually we prepend used regardless of if the attribute is already there, so I think it's just a matter of finding the start of the macro expansion... and making sure the macro doesn't also expand to anything that's not part of the function which might not be totally trivial.

[rinon]

Exactly, I just didn't see that as easy.

[ayrtonm]

@randomPoison we probably won't support changing source code for things that are expanded from macros in general. In this case it seems that redeclaring the function with the used attribute works as expected even if we don't prepend the used attribute to the original definition (I tested it but I'm not sure if it may break with some flags because the functions don't have matching attributes). I may need to do that for #428 so that would also fix this issue.