This is an archive of the discontinued LLVM Phabricator instance.

Make the expression parser work for missing weak symbols
ClosedPublic

Authored by jingham on Jun 27 2019, 6:06 PM.

Details

Summary

lldb wasn't handling weak symbols, so if you were debugging a binary that used a weak symbol, and the symbol was missing, any references to the symbol would result in a symbol not found error. But it is actually legitimate to refer to missing weak symbols, you just have to test them against NULL first.

This patch does three things:

  1. Adds a bit to indicate weakness to Symbol and IsWeak & SetIsWeak accessors
  2. Teaches the ObjectFileMachO to detect weak symbols and mark them appropriately
  3. Gets the symbol -> address lookup in IRExecutionUnit.cpp to mark missing weak symbols as such.

Diff Detail

Repository
rL LLVM

Event Timeline

jingham created this revision.Jun 27 2019, 6:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2019, 6:06 PM
clayborg requested changes to this revision.Jun 28 2019, 1:44 PM

A few nits, but nothing structural. Fix those and this will be good to go.

include/lldb/Symbol/Symbol.h
257 ↗(On Diff #206975)

This will increase the size of Symbol since this extra bit makes it go over 16 bits in size. We can steal a bit away from m_type since it is used to hold lldb::SymbolType and we only have 28 lldb::SymbolType enums. You might need to check that the Swift branch didn't add any.

258 ↗(On Diff #206975)

change to:

m_type : 6;

See above comment.

packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py
24 ↗(On Diff #206975)

We should get ELF support for this as well once this is in.

source/Expression/IRInterpreter.cpp
236 ↗(On Diff #206975)

Init this to something in case function below gets changed and doesn't init it.

source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
436 ↗(On Diff #206975)

init to something.

862 ↗(On Diff #206975)

init to something

1034 ↗(On Diff #206975)

init to something.

This revision now requires changes to proceed.Jun 28 2019, 1:44 PM
jingham updated this revision to Diff 207146.Jun 28 2019, 2:33 PM

Addressed Greg's comments

clayborg accepted this revision.Jun 28 2019, 2:36 PM
This revision is now accepted and ready to land.Jun 28 2019, 2:36 PM
jingham marked 8 inline comments as done.Jun 28 2019, 2:37 PM
jingham added inline comments.
include/lldb/Symbol/Symbol.h
258 ↗(On Diff #206975)

Yup, apparently I can't count... Anyway, swift did add a few but we're still well under 6 bits. I added a comment here saying where m_type gets its values from and in the enum saying it needs to stay under 6 bits.

packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py
24 ↗(On Diff #206975)

I don't know how this works in ELF, but if somebody else wants to try their hand at it, that would be great.

This revision was automatically updated to reflect the committed changes.
jingham marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2019, 2:40 PM