[llvm] r355646 - Work around a module build error on the LLDB incremental green dragon bot.

[David Blaikie via llvm-commits]

On Mon, Mar 11, 2019 at 4:03 PM Adrian Prantl <aprantl at apple.com> wrote:

>
> On Mar 11, 2019, at 3:47 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Mon, Mar 11, 2019 at 3:42 PM Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>>
>> On Mar 11, 2019, at 3:35 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Mon, Mar 11, 2019 at 3:04 PM Adrian Prantl <aprantl at apple.com> wrote:
>>
>>>
>>> On Mar 11, 2019, at 2:09 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>>
>>> On Mon, Mar 11, 2019 at 2:01 PM Adrian Prantl <aprantl at apple.com> wrote:
>>>
>>>> You'll see the sad resolution of this two commits further down. (I
>>>> reverted this one, too). There was a header file with a function body that
>>>> used the LLVM_DEBUG macro (which I believe fundamentally cannot work inside
>>>> a non-textual header)
>>>>
>>>
>>> Right - if it depends on its inclusion state to define DEBUG_TYPE that's
>>> not modular. I assume that's the case? Oh, it looks like it is the case,
>>> that DEBUG_TYPE is defined within this file, so I don't think there's a
>>> problem with using LLVM_DEBUG here? It's not intended to consume the value
>>> of DEBUG_TYPE from its inclusion context.
>>>
>>>
>>> Oh, I had missed that it did define its own DEBUG_TYPE.
>>>
>>>
>>> Perhaps that's invalid in a modular context if it's ever included
>>> somewhere that also defines DEBUG_TYPE? I'm not sure there.
>>>
>>>
>>>> and inside of it it called a dump method, thus pulling the reference to
>>>> the dump method into the link.
>>>>
>>>
>>> Missed a step here - inside of what it called a dump function? & why was
>>> that a problem? It called a dump function unconditionally rather than only
>>> under the condition that the dump function is defined? I'm not sure how
>>> modules would make/break that.
>>>
>>>
>>> Perhaps the LLVM_DEBUG macro is a red herring then and just the presence
>>> of the dump() function in the function body was enough to produce the
>>> dependency on the dump function. The linker error was that that dump()
>>> method was an undefined symbol while linking llvm-lto.
>>>
>>
>> I'm still not sure I follow how the modularity of this file would be
>> related to this problem - can you explain it in further detail, to make
>> sure we're fixing this in the right way/tracking any other bugs this might
>> be indicative of?
>>
>>
>>
>> I don't actually have any further knowledge about how any of this
>> happened. We should be able to reproduce the issue by reverting my removal
>> of the dump() calls, and doing a modular non-LSV build of llvm-lto. I've
>> seen the same symptom before (curiously always with dump methods), and was
>> able to "fix" it every time by moving the offending function body into a
>> .cpp file. I never really bothered to investigate what exactly happened,
>> though, I was mostly concerned with quickly getting the bots running again.
>>
>
> I think it's generally the responsibility of bot owners who want to revert
> patches (or otherwise workaround the bot failures) to justify/explain why
> the change is necessary. I'd really like to have more details here -
> especially if this is coming up again and again (all the more reason to
> make sure the changes are correct).
>
>
> I'll try to be more verbose in the future.
>
>
>
>> Something must have marked the function calling the dump method as used,
>> which clearly also was inaccurate.
>>
>
> The dump macro (
> https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/Compiler.h#L464 )
> marks dump methods as used so they're emitted even though they have no
> callers in the binary, so they'll be there if you need them in a debugger.
>
>
> I think the problem here was that the dump method was part of a static
> library that was not linked by llvm-lto,
>

I guess one question would be why wasn't it linked?


> so the fact that it is marked as always used probably doesn't help.
> Secondly, the function that was calling dump() does not have that
> attribute. What I don't understand is why it got linked.
>

Or, yes, why was the caller linked.

Could you please find out & follow up on the thread on the mailing list to
make sure this is documented/clarified?

- Dave


>
> -- adrian
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190311/986cc788/attachment.html>