[LLVMdev] Inconsistent third field in global_ctors (was Re: [llvm] r214321 - UseListOrder: Visit global values)

[Rafael Avila de Espindola]

What we guarantee for the C API is really fuzzy. Given just the "don't break it" rule, adding any assert would be invalid.

If you are really motivated, start a thread on llvm dev about it. IMHO, we need something on the lines of "don't break an existing user". What this would mean is that we *can* change it, but we have to have good reasons to think that no user will break because of it or we coordinate a transition, like what is going on with removal of the old jit (which *is* breaking the C API)

In this particular case, what is the expected way of creating it with the C API? Manually construct the global? If so, it does look like we have to make the third field optional. If wanted, we could also create an c API specific for the constructors, tell people to use it instead and eventually remove support for the old format.

Sent from my iPhone

> On Jul 31, 2014, at 13:45, "Duncan P. N. Exon Smith" <dexonsmith at apple.com> wrote:
> 
> 
>>> On 2014-Jul-31, at 10:07, Reid Kleckner <rnk at google.com> wrote:
>>> 
>>> On Thu, Jul 31, 2014 at 9:49 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>> Hold on, I'm still not sure what kind of guarantees (restrictions) the C
>>> API imposes.
>>> 
>>> If I implement (2), then -verify will fail if someone (e.g.) adds
>>> `@llvm.global_ctors` with 2 fields.  Does C API stability preclude
>>> making that change?
>> 
>> Now you have reached the crux of it. Nobody really agrees. The verifier doesn't reject the 2-field form yet, because I thought that would be a C API break.
>> 
>>> Assuming it does, are there serious concerns with me implementing (1)
>>> instead?
>> 
>> That was my original intention: this would be an optional field indefinitely, but it means globalopt, asan, and other consumers of global_ctors have to dispatch on the size of the struct forever.
> 
> Well, indefinitely, not forever.  And that's no different from the
> current status, since it's not like they can rely on having been
> serialized to/from bitcode; in particular, I don't think the
> auto-upgrade is actually buying us anything.
> 
> I guess the "right" fix might be to make these global arrays first-class
> IR objects, but even if we had the right design, it's not clear how to
> upgrade to *that*.
> 
> The main complication with (1) is getting ModuleLinker to do the right
> thing -- I think I can just move the canonicalization code there, and
> only trigger it if the constructor styles don't match between the two
> modules.  AFAICT, it's doing the wrong thing right now if the modules
> aren't read from bitcode, so this is probably necessary anyway.
>