[Lldb-commits] [PATCH] D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added inline comments.


================
Comment at: lldb/source/Target/Process.cpp:5569-5570
+
+  if (m_code_address_mask == 0)
+    return -1; // All bits are used for addressing.
+
----------------
jasonmolenda wrote:
> pcc wrote:
> > JDevlieghere wrote:
> > > jasonmolenda wrote:
> > > > pcc wrote:
> > > > > Is this part correct? (Same below.) In D100521 you have
> > > > > ```
> > > > >   if (pc & pac_sign_extension)
> > > > >     return pc | mask;
> > > > >   return pc & (~mask);
> > > > > ```
> > > > > So it looks like this would cause the pc to be set to 0 (or -1)?
> > > > I get confused so I like to do this by hand quickly to make sure I understand.
> > > > 
> > > > given mask of 1110000 and addr of xxx1011 where 'x' are PAC bits,
> > > > 
> > > > b55 == 1: m | a == 1111011
> > > > b55 == 0: ~m & a == 0001011
> > > > 
> > > > given mask of 1111111, low address 0001011 and high address 1111011,
> > > > 
> > > > b55 == 1: m & ha == ha
> > > > b55 == 0: ~m | la == la
> > > > 
> > > > am I  not thinking of something that could unify these?  I can confuse myself so easily with these things.
> > > > 
> > > > We could also detect a mask of -1 and just return the original address in FixCodeAddress/FixDataAddress, right.  That would be very simple.
> > > I've added checking for -1 in D100521
> > With a mask of 1111111 isn't it
> > 
> > b55 == 1: m | ha == 1111111
> > b55 == 0: ~m & la == 0000000
> > 
> > I think you can just remove lines 5569-5570 here as well as lines 819-820 from D100521.
> urk.  my caveat that I often confuse myself with these has been proven true. :) 
> 
> So you're suggesting the default, uninitialized, mask is 0.  If we have that, then
> 
> mask 0, low address 0001011, high address 1111011
> b55 == 1: m | ha == 1111011
> b55 == 0: ~m & la == 0001011
> 
> In the mask, any bit set to 0 is passed through as-is.  Any bit set to 1 is going to be cleared or set in these FixAddress methods.  I see.
Yes, exactly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100515/new/

https://reviews.llvm.org/D100515