mstorsjo marked an inline comment as done.
mstorsjo added a comment.

In D70848#1768730 <>, @labath wrote:

> BTW, I don't know if you've noticed, but the new test is failing on windows: 
> It 
> looks like a path separator problem. We actually already have code which 
> tries to guess the path style of a compile unit, but it is keyed off of the 
> DW_AT_comp_dir attribute, which it looks like you stripped from the test 
> case. My guess is that adding this attribute back will fix this problem...

I did notice it, and tried to fix it 
(, but I forgot to 
check that it actually worked - sorry about that. Apparently I got the 
backslash escaping wrong (from some clang test). I'll see if DW_AT_comp_dir 
helps (and remove that failed regex fix), or fix the backslash matching.

Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486
+  size_t ret = data.SetData(m_data, offset, length);
+  // DataExtractor::SetData copies the address byte size from m_data, but
+  // m_data's address byte size is only set from sizeof(void*), and we can't
+  // access subclasses GetAddressByteSize() when setting up m_data in the
+  // constructor.
+  data.SetAddressByteSize(GetAddressByteSize());
+  return ret;
labath wrote:
> mstorsjo wrote:
> > clayborg wrote:
> > > I would vote to make this happen within DataExtractor::SetData(const 
> > > DataExtractor &...)
> > Do you mean that we'd extend `DataExtractor::SetData(const DataExtractor 
> > &...)` to take a byte address size parameter, or that we'd update 
> > `m_data`'s byte address size before doing `data.SetData()` here?
> > 
> > Ideally I'd set the right byte address size in `m_data` as soon as it is 
> > known and available. It's not possible to do this in `ObjectFile`'s 
> > constructor, as that is called before the subclass is constructed and its 
> > virtual methods are available, but is there a better point in the lifetime 
> > where we could update it?
> I too would prefer that, but I couldn't see a way to achieve that (which is 
> why I stamped this version).
> Today, with a fresh set of eyes, I think it may be reasonable to have this 
> happen in `ObjectFile::ParseHeader`. After the header has been parsed, all 
> object formats (I hope) should be able to determine their address size and 
> endianness, and the operating invariant would be that the address size is 
> only valid after the ParseHeader has been called. WDYT?
Sounds sensible! I'll give it a shot.

  rG LLVM Github Monorepo


lldb-commits mailing list

Reply via email to