[lldb][PPC64] Fixed step-in stopping in the wrong line

[luporl]

Summary

LLDB was not handling properly PPC64 functions that were entered using the local entry point, instead of the global one.

This was fixed by introducing a new function in the Architecture class, with an implementation for PPC64 that is able to properly handle the "skip past prologue" part of step-in.

There was also another related issue found: setting function breakpoints in PPC64. For functions with more than one entry point, the breakpoint was always being placed in function's global entry point, that would not be hit when the function was entered through a local context.
This was also fixed with a new Architecture function, that lets the plugin implementation to adjust the breakpoint address, which PPC64 uses to move the it to the local entry point, that is guaranteed to be hit even when entering the function through the global entry point.

[clayborg]

This seems like functionality that should go into an architecture plug-in. The class is currently "lldb_private::Architecture". For an example see the ARM version:

lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h
lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp

We will need to add a new virtual call to lldb_private::Architecture with a default implementation that does nothing. Then you can add a PPC plug-in and do this work there.

You would get hold of the architecture plugin via the target:

Architecture *architecture = target->GetArchitecturePlugin();
if (architecture) { // Not all architectures have plug-ins, just ARM and soon PPC
  bytes_to_skip = architecture->GetBytesToSkip(this);

[clayborg]

Added Jim Ingham so he can chime in on this and see if he agrees with my solution

[jingham]

I'd go a bit further than Greg, and say the Function should have an API like (might return a reference if the Function holds onto the vector):

struct EntryPointInfo {
    lldb_addr_t entry_point_addr;
    size_t prologue_size;
};

std::vector<EntryPointInfo> Function::GetEntryPointInfo()

then the logic in the step in plan could iterate over the vector, comparing the pc to the entry_point_address and skip that entry points prologue size.

That would also alleviate one of my worries about the proposed solution, which is that it assumes that all local entry points have the same prologue size. That might be true in this case but I can't see why it is necessarily true. There are some languages that actually support alternate entry points for functions (none that we currently support but still...) which very well might do different amounts of setup work for the frame. So it wouldn't hurt to do this generally here in the stepping logic.

[jingham]

BTW, do you also need to handle breakpoint setting here? If you set a breakpoint on a name, does it get both the global & local entry point? Or do you always go through one or the other? And again, when pushing the breakpoint past the prologue, do we need to handle the different entry points differently? I don't know why these local entry points are there, so maybe this question is off base, but abstractly it seems like you would have to worry about that.

[luporl]

In D42582#989512, @jingham wrote:

BTW, do you also need to handle breakpoint setting here? If you set a breakpoint on a name, does it get both the global & local entry point? Or do you always go through one or the other? And again, when pushing the breakpoint past the prologue, do we need to handle the different entry points differently? I don't know why these local entry points are there, so maybe this question is off base, but abstractly it seems like you would have to worry about that.

Yes, I probably also need to handle breakpoint setting here. Right now, breakpoints are set only in the global entry point. But it would be better to set breakpoints at both global & local entry points, to avoid loosing local calls. However, this could cause the program to stop twice, in the case of entering through the local entry point. Unless we always skip the prologue after hitting either breakpoint, then it should be ok.

About the bytes that have to be skipped, at least for PPC64, they will always be the same, as long as you add it to the function start address (a.k.a. global entry point), that is what was already being done in the code.
If it needs to be calculated based on the current address instead, then we just need to subtract the local entry point offset from the prologue byte size.

I think the PPC64 assembly code below may help in understanding a little better its global & local function entry points:

#  template<typename T> T
#  max_value(const T& lhs, const T& rhs)
#  {
#      return std::max(lhs, rhs); // In max_value template
#  }
# (code for T = int)

    .section    .text._Z9max_valueIiET_RKS0_S2_,"axG",@progbits,_Z9max_valueIiET_RKS0_S2_,comdat
    .weak    _Z9max_valueIiET_RKS0_S2_ # -- Begin function _Z9max_valueIiET_RKS0_S2_
    .p2align    4
    .type    _Z9max_valueIiET_RKS0_S2_,@function
_Z9max_valueIiET_RKS0_S2_:              # @_Z9max_valueIiET_RKS0_S2_
.Lfunc_begin9:
    .loc    1 104 0 is_stmt 1       # calling.cpp:104:0
    .cfi_startproc
.Lfunc_gep9:
    addis 2, 12, .TOC.-.Lfunc_gep9@ha
    addi 2, 2, .TOC.-.Lfunc_gep9@l
.Lfunc_lep9:
    .localentry    _Z9max_valueIiET_RKS0_S2_, .Lfunc_lep9-.Lfunc_gep9
# %bb.0:                                # %entry
    mflr 0
    std 31, -8(1)
    std 0, 16(1)
    stdu 1, -128(1)
    .cfi_def_cfa_offset 128
    .cfi_offset r31, -8
    .cfi_offset lr, 16
    mr 31, 1
    .cfi_def_cfa_register r31
    std 3, 112(31)
    std 4, 104(31)
.Ltmp36:
    .loc    1 105 21 prologue_end   # calling.cpp:105:21
    ld 3, 112(31)
    .loc    1 105 26 is_stmt 0      # calling.cpp:105:26
    ld 4, 104(31)
    .loc    1 105 12                # calling.cpp:105:12
    bl _ZSt3maxIiERKT_S2_S2_
    nop
    lwa 3, 0(3)
    .loc    1 105 5                 # calling.cpp:105:5
    addi 1, 1, 128
    ld 0, 16(1)
    ld 31, -8(1)
    mtlr 0
    blr
.Ltmp37:
    .long    0
    .quad    0
.Lfunc_end9:
    .size    _Z9max_valueIiET_RKS0_S2_, .Lfunc_end9-.Lfunc_begin9
    .cfi_endproc

.Lfunc_gep9 is the global entry point, that will always match with address of the function symbol.

.Lfunc_lep9 is the local entry point, that, AFAIK, will always be at the start address of the function plus a byte offset corresponding to instructions that should be skipped when entering the function locally.
In this example, it is skipping the 2 instructions that set the TOC (Table Of Contents), as it will already have been set to the correct value if this is a local call (from a module that shares the same TOC).
The .localentry directive is what instructs the assembler to save the local entry point offset in the st_other field of the function symbol in an ELF file.

[jingham]

I see, thanks for the explanation of how things work on PPC.

Both Greg and my objection was that the code you are having to insert into the generic stepping algorithm - and will have to reproduce in the breakpoint setting/push past prologue code seems way too platform specific to be where it is in this patch (and will have to be for breakpoints as well.) For instance, the thread plans have no business knowing that the all the entry points lie within the prologue of the main entry point, so you can always use that.

So we need to come up with a good abstraction for it. In this case, it looks like you can figure out from the Symbol what the alternate entry points are, so my suggestion of putting it on the Function was wrong, it should be on the symbol. But that seems like the reasonable actor to ask this question. Then the Symbol's probably should ask the ABI (that was Greg's original suggestion.)

[luporl]

Moved PPC64 specific code to a new ArchitecturePPC64 plugin.

Following Greg's and Jim Ingham's suggestions, a new GetBytesToSkip()
virtual Architecture method was added, to handle PPC64's global and
local entry points when LLDB needs to know how many bytes to skip to
get past the prologue.

I'm not sure if this matches exactly what was suggested, but I hope it's close
enough. I added a few parameters to the method, to avoid duplicating code from
ThreadPlanStepInRange. I also have made the default implementation return
LLDB_INVALID_OFFSET, in case there is an Architecture plugin but it doesn't
need/want to implement this method
(otherwise step-in would probably stop working on ARM).

• Merge branch 'master' into fix-step-in
• Moved changes to new ArchitecturePPC64 plugin
• Fixed lldb-server aborting on no arch found assert
• Fixed ArchitecturePPC64 initialization

[clayborg]

Looks good! Not sure if we need the PPC specific comment in Architecture.h, but I won't hold up this patch for that. Might be nice to move the PPC comment into ArchitecturePPC64.h.

[jingham]

This fix is fine for this part of the multiple entry points problem. My guess is that when you start dealing breakpoints and multiple entry points this method will start to look ugly, and having Symbols just return their multiple entry points & prologue length from each will look nicer. But we can address that when you get that far.

There was one comment related comment, otherwise this looks okay.

include/lldb/Core/Architecture.h

53–55

I don't mind using the PPC64 case as an example. But the sentence starting with "Thus..." actually defines how the API should work, so it should be moved outside the example paragraph or it's likely to be overlooked.

[luporl]

• Changed GetBytesToSkip comment to focus on the API

Now the first two paragraphs explain the API.
The last one is the PPC64 example.
I also moved some more specific PPC64 details to the ArchitecturePPC64 header,
making the example in the base Architecture class a bit simpler.

As @jingham pointed out, this method may need some refactoring when dealing with
breakpoints and multiple entry points. But when this time comes, we will
probably have a better view of how to improve this method.

[jingham]

Looks fine.

One other point, I'm assuming you started this whole process because you saw a test case failure and went to fix it? If that's true, do you know how stable that failure was (was it overly dependent on accidents of the compiler or was it pretty clear the function we stepped too far past was always going to have two entry points & so cause the problem? If there was something we could do to make sure the test case doesn't fail to catch the error, that would help ensure this didn't regress.

Of course if you just did this speculatively not because of a test failure, we probably should have a test.

[luporl]

That's right, there was a test case that was failing and I wanted to fix it (TestInlineStepping).
The failure was pretty stable. From what I've seen so far, these are the two conditions that, if met, will cause a failure if the local entry point is not handled:

1. The function needs to have external linkage.
2. The function needs to be called locally (from the same module/compilation unit).

Having external linkage means the compiler needs to generate a global entry point for the function to be called from other modules.
Otherwise, it may have only one entry point (it would be technically a local one, but as it would be the only one it would look more like the global one, that skips nothing).
Also, if the function is not used locally, then the compiler may generate only the global entry point, that was already handled correctly by previous code, which also implies that the test must call the function from the same module to end up at the local entry point.

[clayborg]

Question: if you have a global entry point, is there ever any reason to stop at these? Is there anything you can debug in the global entry point? Does a global entry point always forward on to a local entry point? Does the local entry point always exists and is it the only thing that can be debugged? If so, then it sounds like we should mark the global entry point as a trampoline by setting the symbol type to lldb::eSymbolTypeTrampoline. Then all stepping logic will automatically just go through this code.

[luporl]

if you have a global entry point, is there ever any reason to stop at these? Is there anything you can debug in the global entry point?

You can look at it as part of the prologue. You'll want to debug it only if you want to debug the prologue.

Does a global entry point always forward on to a local entry point?

When there are 2 entry points, yes, the global entry point always forwards to the local entry point.

Does the local entry point always exists and is it the only thing that can be debugged?

Some functions may have only one entry point, be it local or global. If you don't want to debug the prologue, you will actually want to skip some bytes past the local entry point, to get past the prologue, to code that can be debugged.

Overall you can look at the local entry point as a kind of optimization, that enables a local call to skip part of the prologue.

[luporl]

• Revert "Changed GetBytesToSkip comment to focus on the API"
• Revert "Fixed ArchitecturePPC64 initialization"
• Revert "Moved changes to new ArchitecturePPC64 plugin"
• Revert "Fixed step-in stopping in the wrong line in PPC64"
• Handling PPC64 local entry point with extra symbol
• Merge branch 'master' into fix-step-in2

This new change is an attempt to handle PPC64 global/local entry points with
trampoline symbols.

This "works" for step in, but it doesn't look good and I had to deviate enough
from the last suggestion from Jim in order to make it work (see item (3)
below). Or perhaps it was I that didn't understand it correctly.

The following are descriptions of what did not work:

1- Set the global entry point (GEP) as a trampoline to the local entry point

(LEP)

Issues:

• Although the forwarding to the LEP works, LLDB doesn't know how to handle a "LEP", that is different than functions entered from the other trampoline symbols in LLDB.
• The address from the LEP won't match the debug information of where the function starts, which causes issues in step in logic and prologue size calculation.
• It also messes up with the disassembly of the function: instructions will start at a non-zero offset from the function symbol (because the function symbol is always equal to GEP address).

2- Set both entry points as trampolines to the prologue end

(to be able to handle "skip past prologue" correctly)

Issues:

• Trampoline symbols don't work well with GetPrologueSize() code.
• To try to make this work without too much effort, it would be needed yet another symbol at prologue end, and custom code to find the prologue end.
• This would probably have other side effects, as with unwind assembly, for instance, that would get a function without prologue to process.

This is what worked (this change):

3- Setting the LEP as a trampoline (actually, as a local symbol) to prologue

end

To avoid the issues above, it looks like the best way is to avoid modifying
the original function symbol (a.k.a. the GEP).
Also note that step in already works when the function is entered by the
global entry point. For most purposes, it's just like a regular function
start address, that is already handled correctly by LLDB.

Issues:

• The LEP trampoline gets shadowed by the function symbol, which causes the DynamicLoader to not know that it's in its address by regular means. To work around this, the code needs to perform several steps to look for the LEP symbol, whenever it could be have been shadowed by the GEP, which can be expensive.
• Trampoline symbols are skipped from name lookup, so for the bullet above to work, the symbol type must be something else.

In the end, it seems to me that trying to handle PPC64 GEP/LEP with
trampolines causes more trouble than it solves.

IMHO, what we need is a clean way to check if a function has more than one
entry point, and to get its address. Then we could just make some small
changes to code parts that may need to consider more than one entry point,
such as the step in plan, that in this case would just need to perform an
additional check to see if the current address matches any known entry point.

[clayborg]

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

464–493 ↗

(On Diff #135303)

This seems like something that needs to go into the architecture plug-in where the plugin uses just the public APIs on lldb_private::Process/Thread/StackFrame/RegisterContext to do its thing.

503–505 ↗

(On Diff #135303)

Grab arch plugin after moving HandlePPC64LocalEntryPoint to PPC64 arch plug-in.

[luporl]

• Restored PPC64 Architecture Plugin
• Restored/refactored GetBytesToSkip() method, to present a cleaner API (this replaces the trampoline code attempt, that didn't work very well)
• Added an AdjustBreakpointAddress method, to fix issues with setting breakpoints in functions with multiple entry points

[clayborg]

I am good with this as long as Jim and Pavel are.

[labath]

I don't think I know enough about this, so I'll defer to Jim to comment on the design. I just have some comments about the implementation.

Also, it looks like at least the breakpoint-setting part should be easy to test in a freestanding environment: make a .s file which sets up the function, llvm-mc it, load it up in lldb, set a breakpoint, verify it got moved to the right offset. I've been playing around with a patch that should make this easier for you. I'll see if I can clean it up and upload today...

include/lldb/Core/Architecture.h

63

It looks like symbol is always going to be non-null here, right? Can we make this a reference? maybe even a const reference?

source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp

58–61

Should we move this check into the Create function? I know this plugin is kinda supposed to be OS-agnostic, but if we're going to be checking the object format anyway, we might as well do it early-on. We can revisit that once we have a non-elf customer who wants a ppc64 architecture plugin.

[jingham]

GetBytesToSkip strikes me as an odd function. It takes a thread but implicitly acts on the PC of frame 0 of that thread. That makes it feel overly specific and requires a process when you don't really need one to do the calculation. Plus you already have the pc & symbol but you end up re-looking them up. You could do all the calculations you do in GetBytesToSkip if you passed in the symbol and the pc as an Address. That would allow you to get the file address for the PC, and you can compare that directly to the symbol's address.

You can get an Address from a load address (which curr_addr is) by calling target->GetSectionLoadList()->ResolveLoadAddress(curr_addr, ReturnedAddress). Then you can use the file address from the ReturnedAddress to compare against the FileAddress of the symbol.

The way you are using these is fine, BTW. I'm just quibbling about the interface of GetBytesToSkip.

[luporl]

In D42582#1017455, @labath wrote:

I don't think I know enough about this, so I'll defer to Jim to comment on the design. I just have some comments about the implementation.

Also, it looks like at least the breakpoint-setting part should be easy to test in a freestanding environment: make a .s file which sets up the function, llvm-mc it, load it up in lldb, set a breakpoint, verify it got moved to the right offset. I've been playing around with a patch that should make this easier for you. I'll see if I can clean it up and upload today...

Do you think we need a separate test for this? There are already at least 2 tests that always fail without the breakpoint fix: TestDeadStrip and TestBacktraceAll.

[luporl]

include/lldb/Core/Architecture.h

63

Right, we can make this a const reference.

source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp

58–61

Yes, at least for now it would be better to do this check right at creation time.

[luporl]

In D42582#1018105, @jingham wrote:

GetBytesToSkip strikes me as an odd function. It takes a thread but implicitly acts on the PC of frame 0 of that thread. That makes it feel overly specific and requires a process when you don't really need one to do the calculation. Plus you already have the pc & symbol but you end up re-looking them up. You could do all the calculations you do in GetBytesToSkip if you passed in the symbol and the pc as an Address. That would allow you to get the file address for the PC, and you can compare that directly to the symbol's address.

You can get an Address from a load address (which curr_addr is) by calling target->GetSectionLoadList()->ResolveLoadAddress(curr_addr, ReturnedAddress). Then you can use the file address from the ReturnedAddress to compare against the FileAddress of the symbol.

The way you are using these is fine, BTW. I'm just quibbling about the interface of GetBytesToSkip.

Ok, I agree that this will really make a better and easier to understand interface.
I'll make the changes, test it and then upload the new diff.

[luporl]

• Improved GetBytesToSkip() interface
• Performed requested changes

[labath]

In D42582#1019241, @luporl wrote:

In D42582#1017455, @labath wrote:

I don't think I know enough about this, so I'll defer to Jim to comment on the design. I just have some comments about the implementation.

Also, it looks like at least the breakpoint-setting part should be easy to test in a freestanding environment: make a .s file which sets up the function, llvm-mc it, load it up in lldb, set a breakpoint, verify it got moved to the right offset. I've been playing around with a patch that should make this easier for you. I'll see if I can clean it up and upload today...

Do you think we need a separate test for this? There are already at least 2 tests that always fail without the breakpoint fix: TestDeadStrip and TestBacktraceAll.

The idea here is to make a test that people that happen to don't have a ppc machine around (i.e. nearly everyone) can run. So yes, I think a extra test is needed, particularly as it looks like it should be easy to write (if not, then I'd at least like to know why). I was thinking of a non-execution test similar to D43686, where you just set a breakpoint and verify it has been resolved to the right file address.

[jingham]

Pavel's requesting a specific test of this, which seems a good idea to me. Besides that, this is a go from my point of view.

[luporl]

• Merge branch 'master' into fix-step-in
• Added break in function local entry point test

Also added a "Where:" output line, to be able to match the breakpoint
location with function symbol plus offset.

[luporl]

@labath, did you have a chance to look at the breakpoint test, and the new "where" line in lldb-test?

[labath]

In D42582#1027372, @luporl wrote:

@labath, did you have a chance to look at the breakpoint test, and the new "where" line in lldb-test?

Sorry, I missed this one. That's exactly the kind of test I had in mind. We just need to clean it up a bit.

lit/Breakpoint/Inputs/ppc64-localentry.s

29–60

as far as i can tell, the main function is not used in the test. Can you remove it?
On the other hand, if you can think of any other interesting cases to test (e.g., can a function *not* have a local entry point?), then it would be nice to add those.

tools/lldb-test/lldb-test.cpp

100–107

The address part is currently not used for anything - I just added it in case it was needed. I think we should just delete it and rename your field to "address". As it stands now, the difference between "where" and "Address" is very unobvious.

[luporl]

lit/Breakpoint/Inputs/ppc64-localentry.s

29–60

Yes, main can be removed. For a moment I forgot that this code would not be executed in the test.

Setting a breakpoint in a function without a local entry point is a good test case. I'll add that.

tools/lldb-test/lldb-test.cpp

100–107

Ok.

[luporl]

• Merge branch 'master' into fix-step-in
• Improved ppc64 local entry point test

[labath]

Thanks.

[luporl]

@jingham, I think this needs your approval to go ahead, right?

[jingham]

Checking the box...

[luporl]

Thank you @labath, @clayborg and @jingham. Can you please check this in for me?

[jingham]

Done (r327318). Thanks again for the patch!

[aprantl]

It looks like you forgot to git-add source/Plugins/Architecture/PPC64 ?

[vsk]

This appears to cause configuration issues with the cmake and Xcode builds on Darwin.

CMake Error at tools/lldb/source/Plugins/Architecture/CMakeLists.txt:2 (add_subdirectory):
  add_subdirectory given source "PPC64" which is not an existing directory.


-- Symbols (liblldb): exporting all symbols from the lldb namespace
-- Path to the lldb debugserver: /Volumes/Xcode9E129_m17E168_i15E197_t15L194_w15T195_b15P3158_XcodeInternals_32bitSupport_FastSim_Boost_ASan_29GB/Xcode.app/Contents/Developer/../SharedFrameworks/LLDB.framework/Resources/debugserver
-- Configuring incomplete, errors occurred!
See also "/Users/vsk/src/builds/llvm.org-lldbsan-RA/CMakeFiles/CMakeOutput.log".
See also "/Users/vsk/src/builds/llvm.org-lldbsan-RA/CMakeFiles/CMakeError.log".
FAILED: build.ninja
/Volumes/Xcode9E129_m17E168_i15E197_t15L194_w15T195_b15P3158_XcodeInternals_32bitSupport_FastSim_Boost_ASan_29GB/Xcode.app/Contents/Developer/usr/local/bin/cmake -H/Users/vsk/src/llvm.org-lldbsan/llvm -B/Users/vsk/src/builds/llvm.org-lldbsan-RA
ninja: error: rebuilding 'build.ninja': subcommand failed

CompileC build/lldb.build/Release/lldb-core.build/Objects-normal/x86_64/ArchitecturePPC64.o source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
    cd /Users/buildslave/jenkins/workspace/lldb-xcode/lldb
    export LANG=en_US.US-ASCII
    /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -x c++ -arch x86_64 -fmessage-length=0 -fdiagnostics-show-note-include-stack -fmacro-backtrace-limit=0 -std=c++11 -stdlib=libc++ -Wno-trigraphs -fpascal-strings -Os -fno-common -Wmissing-field-initializers -Wno-missing-prototypes -Wunreachable-code -Wnon-virtual-dtor -Woverloaded-virtual -Wno-exit-time-destructors -Wmissing-braces -Wparentheses -Wswitch -Wunused-function -Wunused-label -Wno-unused-parameter -Wunused-variable -Wunused-value -Wempty-body -Wuninitialized -Wno-unknown-pragmas -Wno-shadow -Wno-four-char-constants -Wno-conversion -Wconstant-conversion -Wint-conversion -Wbool-conversion -Wenum-conversion -Wno-shorten-64-to-32 -Wnewline-eof -Wno-c++11-extensions -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -DLLDB_CONFIGURATION_RELEASE -DLLDB_VERSION_STRING=lldb-360.99.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.12 -g -Wno-sign-conversion -Wno-infinite-recursion -Wno-move -iquote /Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/liblldb-core-generated-files.hmap -I/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/liblldb-core-own-target-headers.hmap -I/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/liblldb-core-all-target-headers.hmap -iquote /Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/liblldb-core-project-headers.hmap -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/include -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/source -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/llvm/include -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/llvm/tools/clang/include -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/llvm-build/Release+Asserts/x86_64/include -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/llvm-build/Release+Asserts/x86_64/tools/clang/include -iquote/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/llvm-build/Release+Asserts/x86_64/lib/Target/ARM -I/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/Release/include -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/libxml2 -I/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/DerivedSources/x86_64 -I/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/DerivedSources -Wreorder -F/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/Release -F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/PrivateFrameworks -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -fno-rtti -Wglobal-constructors -Wparentheses -DHAVE_LIBZ=1 -Wimplicit-fallthrough -DLLDB_USE_BUILTIN_DEMANGLER -DLIBXML2_DEFINED -MMD -MT dependencies -MF /Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/Objects-normal/x86_64/ArchitecturePPC64.d --serialize-diagnostics /Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/Objects-normal/x86_64/ArchitecturePPC64.dia -c /Users/buildslave/jenkins/workspace/lldb-xcode/lldb/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp -o /Users/buildslave/jenkins/workspace/lldb-xcode/lldb/build/lldb.build/Release/lldb-core.build/Objects-normal/x86_64/ArchitecturePPC64.o
clang: error: no such file or directory: '/Users/buildslave/jenkins/workspace/lldb-xcode/lldb/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp'
clang: error: no input files

[vsk]

I needed to revert this in r327327 to get the bots going again.

[jingham]

I didn't notice that a directory got added. I re-submitted it and everything seems good now.

[luporl]

Closed by commit rL327331: Re-add change for https://reviews.llvm.org/D42582 with added directories.

include/lldb/Core/Architecture.h

public:
   /// This is specifically used for ARM in Thumb code when we stop in
   /// an IT instruction (if/then/else) where the instruction won't get
   /// executed and therefore it wouldn't be correct to show the program
   /// stopped at the current PC. The code is generic and applies to all
   /// ARM CPUs.
   //------------------------------------------------------------------
   virtual void OverrideStopInfo(Thread &thread) = 0;
 
+  //------------------------------------------------------------------
+  /// This method is used to get the number of bytes that should be
+  /// skipped, from function start address, to reach the first
+  /// instruction after the prologue. If overrode, it must return
+  /// non-zero only if the current address matches one of the known
+  /// function entry points.
+  ///
+  /// This method is called only if the standard platform-independent
+  /// code fails to get the number of bytes to skip, giving the plugin
+  /// a chance to try to find the missing info.
+  ///
+  /// This is specifically used for PPC64, where functions may have
+  /// more than one entry point, global and local, so both should
+  /// be compared with current address, in order to find out the
+  /// number of bytes that should be skipped, in case we are stopped
+  /// at either function entry point.
+  //------------------------------------------------------------------
+  virtual size_t GetBytesToSkip(Symbol &func, const Address &curr_addr) const {
+    return 0;
+  }
+
+  //------------------------------------------------------------------
+  /// Adjust function breakpoint address, if needed. In some cases,
+  /// the function start address is not the right place to set the
+  /// breakpoint, specially in functions with multiple entry points.
+  ///
+  /// This is specifically used for PPC64, for functions that have
+  /// both a global and a local entry point. In this case, the
+  /// breakpoint is adjusted to the first function address reached
+  /// by both entry points.
+  //------------------------------------------------------------------
+  virtual void AdjustBreakpointAddress(const Symbol &func,
+                                       Address &addr) const {}
+
 private:
   Architecture(const Architecture &) = delete;
   void operator=(const Architecture &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_CORE_ARCHITECTURE_H

lit/Breakpoint/Inputs/ppc64-localentry.s

+  .text
+  .abiversion 2
+
+  .globl  lfunc
+  .p2align  4
+  .type lfunc,@function
+lfunc:                                  # @lfunc
+.Lfunc_begin0:
+.Lfunc_gep0:
+  addis 2, 12, .TOC.-.Lfunc_gep0@ha
+  addi 2, 2, .TOC.-.Lfunc_gep0@l
+.Lfunc_lep0:
+  .localentry lfunc, .Lfunc_lep0-.Lfunc_gep0
+# BB#0:
+  mr 4, 3
+  addis 3, 2, .LC0@toc@ha
+  ld 3, .LC0@toc@l(3)
+  stw 4, -12(1)
+  lwz 4, 0(3)
+  lwz 5, -12(1)
+  mullw 4, 4, 5
+  extsw 3, 4
+  blr
+  .long 0
+  .quad 0
+.Lfunc_end0:
+  .size lfunc, .Lfunc_end0-.Lfunc_begin0
+
+  .globl  simple
+  .p2align  4
+  .type simple,@function
+simple:                                 # @simple
+.Lfunc_begin1:
+# %bb.0:                                # %entry
+  mr 4, 3
+  stw 4, -12(1)
+  lwz 4, -12(1)
+  mulli 4, 4, 10
+  extsw 3, 4
+  blr
+  .long 0
+  .quad 0
+.Lfunc_end1:
+  .size simple, .Lfunc_end1-.Lfunc_begin1
+
+  .section  .toc,"aw",@progbits
+.LC0:
+  .tc g_foo[TC],g_foo
+  .type g_foo,@object           # @g_foo
+  .data
+  .globl  g_foo
+  .p2align  2
+g_foo:
+  .long 2                       # 0x2
+  .size g_foo, 4

lit/Breakpoint/ppc64-localentry.test

+# REQUIRES: powerpc
+#
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj %p/Inputs/ppc64-localentry.s -o %t
+# RUN: lldb-test breakpoints %t %s | FileCheck %s
+
+breakpoint set -n lfunc
+# CHECK-LABEL: breakpoint set -n lfunc
+# CHECK: Address: {{.*}}`lfunc + 8
+
+breakpoint set -n simple
+# CHECK-LABEL: breakpoint set -n simple
+# CHECK: Address: {{.*}}`simple

lit/lit.cfg

 if re.search(r'DEBUG', llvm_config_output_list[0]):
     config.available_features.add('debug')
 if re.search(r'ON', llvm_config_output_list[1]):
     config.available_features.add('asserts')
 if re.search(r'ARM', llvm_config_output_list[2]):
     config.available_features.add('arm')
 if re.search(r'Mips', llvm_config_output_list[2]):
     config.available_features.add('mips')
+if re.search(r'PowerPC', llvm_config_output_list[2]):
+    config.available_features.add('powerpc')
 if re.search(r'X86', llvm_config_output_list[2]):
     config.available_features.add('x86')
 llvm_config_cmd.wait()

source/API/SystemInitializerFull.cpp

 #include "Plugins/ABI/SysV-i386/ABISysV_i386.h"
 #include "Plugins/ABI/SysV-mips/ABISysV_mips.h"
 #include "Plugins/ABI/SysV-mips64/ABISysV_mips64.h"
 #include "Plugins/ABI/SysV-ppc/ABISysV_ppc.h"
 #include "Plugins/ABI/SysV-ppc64/ABISysV_ppc64.h"
 #include "Plugins/ABI/SysV-s390x/ABISysV_s390x.h"
 #include "Plugins/ABI/SysV-x86_64/ABISysV_x86_64.h"
 #include "Plugins/Architecture/Arm/ArchitectureArm.h"
+#include "Plugins/Architecture/PPC64/ArchitecturePPC64.h"
 #include "Plugins/Disassembler/llvm/DisassemblerLLVMC.h"
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h"
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h"
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
#endif
   ABISysV_x86_64::Initialize();
   ABISysV_ppc::Initialize();
   ABISysV_ppc64::Initialize();
   ABISysV_mips::Initialize();
   ABISysV_mips64::Initialize();
   ABISysV_s390x::Initialize();
 
   ArchitectureArm::Initialize();
+  ArchitecturePPC64::Initialize();
 
   DisassemblerLLVMC::Initialize();
 
   JITLoaderGDB::Initialize();
   ProcessElfCore::Initialize();
   minidump::ProcessMinidump::Initialize();
   MemoryHistoryASan::Initialize();
   AddressSanitizerRuntime::Initialize();

source/Breakpoint/BreakpointResolverName.cpp

 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Breakpoint/BreakpointResolverName.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
+#include "lldb/Core/Architecture.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 BreakpointResolverName::BreakpointResolverName(
     Breakpoint *bkpt, const char *name_cstr, uint32_t name_type_mask,
for (i = 0; i < func_list.GetSize(); i++) {
             break_addr = sc.symbol->GetAddress();
           }
 
           if (m_skip_prologue && break_addr.IsValid()) {
             const uint32_t prologue_byte_size =
                 sc.symbol->GetPrologueByteSize();
             if (prologue_byte_size)
               break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+            else {
+              Architecture *arch =
+                  m_breakpoint->GetTarget().GetArchitecturePlugin();
+              if (arch)
+                arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
+            }
           }
         }
 
         if (break_addr.IsValid()) {
           if (filter.AddressPasses(break_addr)) {
             BreakpointLocationSP bp_loc_sp(
                 AddLocation(break_addr, &new_location));
             bp_loc_sp->SetIsReExported(is_reexported);

source/Plugins/Architecture/CMakeLists.txt

 add_subdirectory(Arm)
+add_subdirectory(PPC64)

source/Plugins/Architecture/PPC64/ArchitecturePPC64.h

+//===-- ArchitecturePPC64.h -------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_PLUGIN_ARCHITECTURE_PPC64_H
+#define LLDB_PLUGIN_ARCHITECTURE_PPC64_H
+
+#include "lldb/Core/Architecture.h"
+
+namespace lldb_private {
+
+class ArchitecturePPC64 : public Architecture {
+public:
+  static ConstString GetPluginNameStatic();
+  static void Initialize();
+  static void Terminate();
+
+  ConstString GetPluginName() override;
+  uint32_t GetPluginVersion() override;
+
+  void OverrideStopInfo(Thread &thread) override {}
+
+  //------------------------------------------------------------------
+  /// This method compares current address with current function's
+  /// local entry point, returning the bytes to skip if they match.
+  //------------------------------------------------------------------
+  size_t GetBytesToSkip(Symbol &func, const Address &curr_addr) const override;
+
+  void AdjustBreakpointAddress(const Symbol &func,
+                               Address &addr) const override;
+
+private:
+  static std::unique_ptr<Architecture> Create(const ArchSpec &arch);
+  ArchitecturePPC64() = default;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_PLUGIN_ARCHITECTURE_PPC64_H

source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp

+//===-- ArchitecturePPC64.cpp -----------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Plugins/Architecture/PPC64/ArchitecturePPC64.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Symbol/Function.h"
+#include "lldb/Symbol/Symbol.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/ArchSpec.h"
+
+#include "llvm/BinaryFormat/ELF.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+ConstString ArchitecturePPC64::GetPluginNameStatic() {
+  return ConstString("ppc64");
+}
+
+void ArchitecturePPC64::Initialize() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+                                "PPC64-specific algorithms",
+                                &ArchitecturePPC64::Create);
+}
+
+void ArchitecturePPC64::Terminate() {
+  PluginManager::UnregisterPlugin(&ArchitecturePPC64::Create);
+}
+
+std::unique_ptr<Architecture> ArchitecturePPC64::Create(const ArchSpec &arch) {
+  if ((arch.GetMachine() != llvm::Triple::ppc64 &&
+       arch.GetMachine() != llvm::Triple::ppc64le) ||
+      arch.GetTriple().getObjectFormat() != llvm::Triple::ObjectFormatType::ELF)
+    return nullptr;
+  return std::unique_ptr<Architecture>(new ArchitecturePPC64());
+}
+
+ConstString ArchitecturePPC64::GetPluginName() { return GetPluginNameStatic(); }
+uint32_t ArchitecturePPC64::GetPluginVersion() { return 1; }
+
+static int32_t GetLocalEntryOffset(const Symbol &sym) {
+  unsigned char other = sym.GetFlags() >> 8 & 0xFF;
+  return llvm::ELF::decodePPC64LocalEntryOffset(other);
+}
+
+size_t ArchitecturePPC64::GetBytesToSkip(Symbol &func,
+                                         const Address &curr_addr) const {
+  if (curr_addr.GetFileAddress() ==
+      func.GetFileAddress() + GetLocalEntryOffset(func))
+    return func.GetPrologueByteSize();
+  return 0;
+}
+
+void ArchitecturePPC64::AdjustBreakpointAddress(const Symbol &func,
+                                                Address &addr) const {
+  int32_t loffs = GetLocalEntryOffset(func);
+  if (!loffs)
+    return;
+
+  addr.SetOffset(addr.GetOffset() + loffs);
+}

source/Plugins/Architecture/PPC64/CMakeLists.txt

+add_lldb_library(lldbPluginArchitecturePPC64 PLUGIN
+  ArchitecturePPC64.cpp
+
+  LINK_LIBS
+    lldbPluginProcessUtility
+    lldbCore
+    lldbTarget
+    lldbUtility
+  LINK_COMPONENTS
+    Support
+  )

source/Target/ThreadPlanStepInRange.cpp

 //===-- ThreadPlanStepInRange.cpp -------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Target/ThreadPlanStepInRange.h"
+#include "lldb/Core/Architecture.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlanStepOut.h"
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 
if (!m_sub_plan_sp && frame_order == eFrameCompareYounger &&
         } else if (sc.symbol) {
           func_start_address = sc.symbol->GetAddress();
           if (curr_addr ==
               func_start_address.GetLoadAddress(
                   m_thread.CalculateTarget().get()))
             bytes_to_skip = sc.symbol->GetPrologueByteSize();
         }
 
+        if (bytes_to_skip == 0 && sc.symbol) {
+          TargetSP target = m_thread.CalculateTarget();
+          Architecture *arch = target->GetArchitecturePlugin();
+          if (arch) {
+            Address curr_sec_addr;
+            target->GetSectionLoadList().ResolveLoadAddress(curr_addr,
+                                                            curr_sec_addr);
+            bytes_to_skip = arch->GetBytesToSkip(*sc.symbol, curr_sec_addr);
+          }
+        }
+
         if (bytes_to_skip != 0) {
           func_start_address.Slide(bytes_to_skip);
           log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP);
           if (log)
             log->Printf("Pushing past prologue ");
 
           m_sub_plan_sp = m_thread.QueueThreadPlanForRunToAddress(
               false, func_start_address, true);

tools/lldb-test/SystemInitializerTest.cpp

 #include "Plugins/ABI/SysV-i386/ABISysV_i386.h"
 #include "Plugins/ABI/SysV-mips/ABISysV_mips.h"
 #include "Plugins/ABI/SysV-mips64/ABISysV_mips64.h"
 #include "Plugins/ABI/SysV-ppc/ABISysV_ppc.h"
 #include "Plugins/ABI/SysV-ppc64/ABISysV_ppc64.h"
 #include "Plugins/ABI/SysV-s390x/ABISysV_s390x.h"
 #include "Plugins/ABI/SysV-x86_64/ABISysV_x86_64.h"
 #include "Plugins/Architecture/Arm/ArchitectureArm.h"
+#include "Plugins/Architecture/PPC64/ArchitecturePPC64.h"
 #include "Plugins/Disassembler/llvm/DisassemblerLLVMC.h"
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h"
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h"
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
#endif
   ABISysV_x86_64::Initialize();
   ABISysV_ppc::Initialize();
   ABISysV_ppc64::Initialize();
   ABISysV_mips::Initialize();
   ABISysV_mips64::Initialize();
   ABISysV_s390x::Initialize();
 
   ArchitectureArm::Initialize();
+  ArchitecturePPC64::Initialize();
 
   DisassemblerLLVMC::Initialize();
 
   JITLoaderGDB::Initialize();
   ProcessElfCore::Initialize();
   minidump::ProcessMinidump::Initialize();
   MemoryHistoryASan::Initialize();
   AddressSanitizerRuntime::Initialize();

tools/lldb-test/lldb-test.cpp

P.formatLine("{0} resolved location{1}.", BP->GetNumResolvedLocations(),
                  plural(BP->GetNumResolvedLocations()));
     if (BP->GetNumResolvedLocations() > 0)
       P.formatLine("At least one resolved location.");
     for (size_t l = 0, le = BP->GetNumLocations(); l < le; ++l) {
       BreakpointLocationSP Loc = BP->GetLocationAtIndex(l);
       P.formatLine("Location ID {0}:", Loc->GetID());
       AutoIndent Indent(P, 2);
       P.formatLine("Enabled: {0}", Loc->IsEnabled());
       P.formatLine("Resolved: {0}", Loc->IsResolved());
-      P.formatLine("Address: {0}+{1:x}",
-                   Loc->GetAddress().GetSection()->GetName(),
-                   Loc->GetAddress().GetOffset());
+      SymbolContext sc;
+      Loc->GetAddress().CalculateSymbolContext(&sc);
+      lldb_private::StreamString S;
+      sc.DumpStopContext(&S, BP->GetTarget().GetProcessSP().get(),
+                         Loc->GetAddress(), false, true, false, true, true);
+      P.formatLine("Address: {0}", S.GetString());
     }
   }
   P.NewLine();
 }
 
 std::string opts::breakpoint::substitute(StringRef Cmd) {
   std::string Result;
   raw_string_ostream OS(Result);
   while (!Cmd.empty()) {