Michael137 wrote:

> > FWIW, I came across another no_unique_address-related crash today:
> > ```
> > $ cat a.cc
> > struct S {
> > private:
> >   int i;
> >   short s;
> > };
> > static_assert(sizeof(S) == 8);
> > 
> > struct T {
> >   [[no_unique_address]] S s;
> >   char c;
> > };
> > static_assert(sizeof(T) == 8);
> > 
> > T t;
> > $ clang++ -c -o /tmp/a.out /tmp/a.cc -g
> > $ lldb /tmp/a.out -o "expr t" -x
> > (lldb) target create "/tmp/a.out"
> > Current executable set to '/tmp/a.out' (x86_64).
> > (lldb) expr t
> > assertion failed at clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:960 in void 
> > (anonymous namespace)::CGRecordLowering::checkBitfieldClipping(bool) const: 
> > M.Offset >= Tail && "Bitfield access unit is not clipped"
> > ...
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > I was hoping this PR would fix it, but it doesn't appear to help.
> 
> Hmmm interesting, yea that looks more like something that #96422 should've 
> addressed. Though this is a special case where the attribute is applied to a 
> non-empty type. Would need to step through the `CGRecordLayoutBuilder` to see 
> why Clang believes the offsets from DWARF aren't correct.

Yea I took a quick look at this and the problem is the following check: 
https://github.com/llvm/llvm-project/blob/44be5a7fdc20a7f90d63dc18699a470e900bd3ba/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp#L392

`S` is potentially overlapping because it has the `NoUniqueAddress` attribute, 
but that isn't present in the DWARF AST. So when we compute the record layout 
using the DWARF AST we push_back the incorrect MemberInfo here:
```
(lldb) p getStorageType(Field->getType()->getAsCXXRecordDecl())->dump()         
                                                                                
                                  
%struct.S.base = type <{ i32, i16 }>                                            
                                                                                
                                  
(lldb) p getStorageType(*Field)->dump()                                         
                                                                                
                                                                                
                                                          
%struct.S = type <{ i32, i16, [2 x i8] }>                                       
                          
(lldb) n                                                                        
                          
```
In the `isPotentiallyOverlapping` case we would've picked the layout without 
the tail padding.

So we're back to "how do we encode this isPotentiallyOverlapping property 
DWARF". OR, maybe we can get away with removing that dependency on the AST in 
the same way we did with `isZeroSize`..

https://github.com/llvm/llvm-project/pull/97443
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to