[llvm-commits] cxa_guard elimination

[Eli Friedman]

On Sat, Jan 7, 2012 at 6:48 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> This patch implements some really basic elimination of calls to __cxa_guard.
> More advanced cases can be added on request. Patch attached, please review!
>
> There's one thing I need to draw to a reviewer's attention. I make the
> assumption that the code in the 'not-yet-initialized' path is indeed
> initializing. That is to say that I assume that this pseudo-code:
>
>  *global = x;
>  if (expr) {
>    if (cxa_guard_acquire(global_guard)) {
>      *global = 0;
>      cxa_guard_release(global_guard);
>    }
>  }
>  use(global);
>
> represents undefined behavior because global is initialized before the call
> to __cxa_guard_acquire, and the guard is protecting a normal store, not the
> first store.
>
> The Itanium C++ ABI never states that guard_acquire/release must be used for
> *initializing*, so this patch arguably can miscompile. However, it's
> generally understood that this is for initialization (indeed, the section
> heading is "Once-time initialization API") so I'm hoping we can agree that
> this assumption is valid and ask users doing other things with
> cxa_guard_acquire/release to pass -fno-builtins.

I think I agree with your assumption for the global associated with
the guard.  You can't make that assumption for all globals, though.
Consider something like the following:

static int x;
struct C { C() { x = 10; } };
int f() { x = 20; static C dummy; return x; }

As far as I can tell, with a bit of inlining etc. we end up with code
which looks exactly like your bad pattern.

-Eli