aadsm marked 3 inline comments as done.
aadsm added inline comments.
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:91-94
+ DataExtractor auxv_data(m_process ? m_process->GetAuxvData() :
DataBufferSP(),
+ m_process->GetByteOrder(),
+ m_process->GetAddressByteSize());
+ m_auxv.reset(new AuxVector(auxv_data));
----------------
labath wrote:
> It looks like we're not guarding the call to `LoadModules` on line 101, so I
> think we can assume that m_process is not null here (and I don't see why it
> should be -- it does not make sense to call `DidAttach` if there is no
> process around).
>
> BTW, your guarding attempt isn't even correct as we'd always call
> `m_process->GetByteOrder()` and crash if the process was null.
oh no, this is embarrassing!
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:549-550
- if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
- true /* notify */)) {
+ if (ModuleSP module_sp =
+ target.GetOrCreateModule(module_spec, true /* notify */)) {
UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
----------------
labath wrote:
> Whitespace-only change. Did you maybe run clang-format on the whole file
> instead of just the diff?
>
> It doesn't matter that much here as there isn't many of them, but I just want
> to make sure you're aware of this in the future.
odd, pretty sure I just run clang-format-diff (as I don't have auto formatting
going on), will double check this.
================
Comment at: lldb/source/Plugins/Process/Utility/AuxVector.cpp:38
-AuxVector::AuxVector(Process *process) : m_process(process) {
- DataExtractor data;
- Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
-
- data.SetData(GetAuxvData());
- data.SetByteOrder(m_process->GetByteOrder());
- data.SetAddressByteSize(m_process->GetAddressByteSize());
-
- ParseAuxv(data);
-
- if (log)
- DumpToLog(log);
-}
-
-AuxVector::iterator AuxVector::FindEntry(EntryType type) const {
- for (iterator I = begin(); I != end(); ++I) {
- if (I->type == static_cast<uint64_t>(type))
- return I;
- }
-
- return end();
+uint64_t AuxVector::GetAuxValue(enum EntryType entry_type) const {
+ auto it = m_auxv_entries.find(static_cast<uint64_t>(entry_type));
----------------
labath wrote:
> It would be better to return an Optional<uint64_t> (or addr_t maybe ?)
> instead of using a magic value to mean "not found". It looks like at least
> some of these entries can validly be zero.
Can do, I was trying to follow this api though:
http://man7.org/linux/man-pages/man3/getauxval.3.html.
I think I'll go with the uint64_t though, I find it odd to return an addr_t
because it's not an address, it's a number that happens to be the same size of
an address.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62500/new/
https://reviews.llvm.org/D62500
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
