mstorsjo marked an inline comment as done. mstorsjo added a comment. In D70848#1768730 <https://reviews.llvm.org/D70848#1768730>, @labath wrote:
> BTW, I don't know if you've noticed, but the new test is failing on windows: > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/11217. 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 (https://reviews.llvm.org/rG7d019d1a3be252a885e8db1ee7af11c90), 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70848/new/ https://reviews.llvm.org/D70848 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
