llvm dev - Jun 2014 - [LLVMdev] Issues with clang-llvm debug info validity

Hi all, sorry to post to both lists, but I'm running into an issue where
clang-generated debug info is deemed to be invalid by LLVM tools (throws an
assertion error in both llc and mcjit), and I'm not sure what the proper
resolution is.

Here's a test case; I last tested it on revision r210953:

$ cat test1.cpp
#include "test.h"
test::Test<int> foo1() {
    return test::Test<int>();
}

$ cat test2.cpp
namespace test {
}
#include "test.h"
test::Test<int> foo2() {
    return test::Test<int>();
}

$ cat test.h
namespace test {
template <class T>
class Test {
};
}

$ clang++ -g -emit-llvm test1.cpp -S -o test1.ll
$ clang++ -g -emit-llvm test2.cpp -S -o test2.ll
$ llvm-link test1.ll test2.ll -S -o linked.ll
$ llc linked.ll

The last step raises an assertion error, assuming you have assertions
enabled:
llc: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:953: llvm::DIE*
llvm::DwarfUnit::getOrCreateTypeDIE(const llvm::MDNode*): Assertion `Ty
=resolve(Ty.getRef()) && "type was not uniqued, possible ODR
violation."'
failed

The initial introduction of the "test" namespace in test2.cpp seems to
confuse clang -- test1.cpp and test2.cpp have non-compatible debug
information for the test::Test type.  test1.cpp says that test::Test's
namespace is defined in test.h, and test2.cpp says that test::Test's
namespace is defined in test2.cpp (they both say that the class itself is
defined in test.h).  Regardless of whether LLVM tools should allow this,
this seems like the wrong output for these files?

Another related example, is if we simply defined the test::Test template in
both files (ie, manually executed the #include "test.h") and got rid
of the
test2.cpp "namespace test {}" definition, we'd run into the same
assertion,
without hitting the weird clang behavior with the empty namespace
pre-declaration.

It's probably also worth noting that compiling directly with clang, ie
adding a "int main() {}" function and then doing
$ clang++ -g test1.cpp test2.cpp -o test
works fine.

I'm not a C++ expert and I don't know if these template issues really
are
violations of the ODR, but it feels like this should be allowed, and
somewhere in the llvm toolchain (llvm-link? DwarfUnit.cpp?) this should be
handled.  What do people think?

kmod
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140613/56861d36/attachment.html>

Yep, looks like a legit bug to me. I'll try to reproduce it and reduce
a test case, etc.

(the ODR doesn't require that the two copies of the Test class
template appear on the same line in the same file in two distinct
translation units, it only requires that the two copies have the same
sequence of tokens - which they do in your example)

On Fri, Jun 13, 2014 at 5:33 PM, Kevin Modzelewski <kmod at dropbox.com>
wrote:
> Hi all, sorry to post to both lists, but I'm running into an issue
where
> clang-generated debug info is deemed to be invalid by LLVM tools (throws an
> assertion error in both llc and mcjit), and I'm not sure what the
proper
> resolution is.
>
> Here's a test case; I last tested it on revision r210953:
>
> $ cat test1.cpp
> #include "test.h"
> test::Test<int> foo1() {
>     return test::Test<int>();
> }
>
> $ cat test2.cpp
> namespace test {
> }
> #include "test.h"
> test::Test<int> foo2() {
>     return test::Test<int>();
> }
>
> $ cat test.h
> namespace test {
> template <class T>
> class Test {
> };
> }
>
> $ clang++ -g -emit-llvm test1.cpp -S -o test1.ll
> $ clang++ -g -emit-llvm test2.cpp -S -o test2.ll
> $ llvm-link test1.ll test2.ll -S -o linked.ll
> $ llc linked.ll
>
> The last step raises an assertion error, assuming you have assertions
> enabled:
> llc: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:953: llvm::DIE*
> llvm::DwarfUnit::getOrCreateTypeDIE(const llvm::MDNode*): Assertion `Ty
=> resolve(Ty.getRef()) && "type was not uniqued, possible ODR
violation."'
> failed
>
> The initial introduction of the "test" namespace in test2.cpp
seems to
> confuse clang -- test1.cpp and test2.cpp have non-compatible debug
> information for the test::Test type.  test1.cpp says that test::Test's
> namespace is defined in test.h, and test2.cpp says that test::Test's
> namespace is defined in test2.cpp (they both say that the class itself is
> defined in test.h).  Regardless of whether LLVM tools should allow this,
> this seems like the wrong output for these files?
>
> Another related example, is if we simply defined the test::Test template in
> both files (ie, manually executed the #include "test.h") and got
rid of the
> test2.cpp "namespace test {}" definition, we'd run into the
same assertion,
> without hitting the weird clang behavior with the empty namespace
> pre-declaration.
>
> It's probably also worth noting that compiling directly with clang, ie
> adding a "int main() {}" function and then doing
> $ clang++ -g test1.cpp test2.cpp -o test
> works fine.
>
> I'm not a C++ expert and I don't know if these template issues
really are
> violations of the ODR, but it feels like this should be allowed, and
> somewhere in the llvm toolchain (llvm-link? DwarfUnit.cpp?) this should be
> handled.  What do people think?
>
> kmod

+Adrian & Manman,

Looks like this is a case of non-DIRef references ending up in the IR,
and thus the references not being deduplicated. This could lead to
some of the IR bloat that you guys implemented the DIRef stuff to
reduce/avoid.

The specific issue is that the type is slightly different in the two
TUs (since the namespace scope it's contained within is different in
the two TUs - note the namespace preceeding the header include in
test2.cpp) and the actual DIRef-usage failure is when building
subroutine types. The subroutine types are made with direct references
to types, not with DIRef references to types. This means they won't be
deduplicated/resolved to a single type during linking correctly.

It also leads to an assertion as per the original email. Figured you
guys might want to look into/fix this (I assume the right fix is to
use DIRef more pervasively in the frontend - I'm doing a little
experimenting with clang now to see how that looks).

- David

On Fri, Jun 13, 2014 at 11:08 PM, David Blaikie <dblaikie at gmail.com>
wrote:
> Yep, looks like a legit bug to me. I'll try to reproduce it and reduce
> a test case, etc.
>
> (the ODR doesn't require that the two copies of the Test class
> template appear on the same line in the same file in two distinct
> translation units, it only requires that the two copies have the same
> sequence of tokens - which they do in your example)
>
> On Fri, Jun 13, 2014 at 5:33 PM, Kevin Modzelewski <kmod at
dropbox.com> wrote:
>> Hi all, sorry to post to both lists, but I'm running into an issue
where
>> clang-generated debug info is deemed to be invalid by LLVM tools
(throws an
>> assertion error in both llc and mcjit), and I'm not sure what the
proper
>> resolution is.
>>
>> Here's a test case; I last tested it on revision r210953:
>>
>> $ cat test1.cpp
>> #include "test.h"
>> test::Test<int> foo1() {
>>     return test::Test<int>();
>> }
>>
>> $ cat test2.cpp
>> namespace test {
>> }
>> #include "test.h"
>> test::Test<int> foo2() {
>>     return test::Test<int>();
>> }
>>
>> $ cat test.h
>> namespace test {
>> template <class T>
>> class Test {
>> };
>> }
>>
>> $ clang++ -g -emit-llvm test1.cpp -S -o test1.ll
>> $ clang++ -g -emit-llvm test2.cpp -S -o test2.ll
>> $ llvm-link test1.ll test2.ll -S -o linked.ll
>> $ llc linked.ll
>>
>> The last step raises an assertion error, assuming you have assertions
>> enabled:
>> llc: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:953: llvm::DIE*
>> llvm::DwarfUnit::getOrCreateTypeDIE(const llvm::MDNode*): Assertion `Ty
=>> resolve(Ty.getRef()) && "type was not uniqued, possible
ODR violation."'
>> failed
>>
>> The initial introduction of the "test" namespace in test2.cpp
seems to
>> confuse clang -- test1.cpp and test2.cpp have non-compatible debug
>> information for the test::Test type.  test1.cpp says that
test::Test's
>> namespace is defined in test.h, and test2.cpp says that
test::Test's
>> namespace is defined in test2.cpp (they both say that the class itself
is
>> defined in test.h).  Regardless of whether LLVM tools should allow
this,
>> this seems like the wrong output for these files?
>>
>> Another related example, is if we simply defined the test::Test
template in
>> both files (ie, manually executed the #include "test.h") and
got rid of the
>> test2.cpp "namespace test {}" definition, we'd run into
the same assertion,
>> without hitting the weird clang behavior with the empty namespace
>> pre-declaration.
>>
>> It's probably also worth noting that compiling directly with clang,
ie
>> adding a "int main() {}" function and then doing
>> $ clang++ -g test1.cpp test2.cpp -o test
>> works fine.
>>
>> I'm not a C++ expert and I don't know if these template issues
really are
>> violations of the ODR, but it feels like this should be allowed, and
>> somewhere in the llvm toolchain (llvm-link? DwarfUnit.cpp?) this should
be
>> handled.  What do people think?
>>
>> kmod