[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I'm not as sure about this.  I think the idea was that eSymbolContextEverything would have a value like 0b1111 and so we know only 4 bits (in my example here) are used for the eSymbolContext* enums.  StackFrame's m_flags wants to use the bits above that range for RESOLVED_FRAME_CODE_ADDR, RESOLVED_FRAME_ID_SYMBOL_SCOPE, GOT_FRAME_BASE, RESOLVED_VARIABLES, RESOLVED_GLOBAL_VARIABLES flags.  So it's setting RESOLVED_FRAME_CODE_ADDR to (in my example) 0b10000, leaving those 4 low bits free to indicate the eSymbolContext values.  That is, we would have

0bnnnn0000 start of eSymbolContext range
0bnnnn1111 end of eSymbolContext range
0b0001mmmm RESOLVED_FRAME_CODE_ADDR
0b0010mmmm RESOLVED_FRAME_ID_SYMBOL_SCOPE
0b0011mmmm GOT_FRAME_BASE
0b0100mmmm RESOLVED_VARIABLES
0b0101mmmm RESOLVED_GLOBAL_VARIABLES

The idea is to have two unrelated set of flags in this single m_flags which I am suspicious of how necessary that is, tbh.  In a typical big multi threaded GUI app you might have 20-30 threads with 40-50 stack frames, that's like 1500 StackFrames.  I think we keep the stack frame list from the previous step when stepping, so twice that.  Having an extra uint32_t for 3k StackFrame objects does not concern me.

I agree that having eSymbolContextVariable be outside the range of eSymbolContextEverything breaks this scheme, but I don't think this is the correct fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139066