[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 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).


> 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.

- Dave


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