[Lldb-commits] [PATCH] D147627: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

[Lu Weining via Phabricator via lldb-commits]

SixWeining accepted this revision.
SixWeining added a comment.
This revision is now accepted and ready to land.

LGTM. I think this is a requirement for adding other 32bit arch support.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147627/new/

https://reviews.llvm.org/D147627

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147674: Interpret ESR/FAR bits directly on watchpoint exceptions in debugserver, clarify how watchpoint descriptions in stop packets work

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

One possible criticism I have of the current reason:watchpoint + 
description-up-to-three-integers is that we should add key-value pairs to the 
stop-info packet `wp-address:` `wp-index:` `wp-hit-address` and honestly, 
`silently-continue:` instead of doing an architectural hardcode in lldb.  The 
remote stub is probably in the best position to know if this is a false 
positive watch trigger that needs to be silently moved past, instead of 
depending on "a third integer which isn't within the range of a watched region 
on MIPS". If I wanted to add a 4th field for "silently skip" to the current 
description, now I'm requiring that the stub reporting flag a false hit 
watchpoint needs to provide the first three always.  And maybe it only has the 
one address or something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147674/new/

https://reviews.llvm.org/D147674

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

In D147606#4246832 , @JDevlieghere 
wrote:

> The change looks fine and regardless of whether this makes sense or even 
> complies with the standard, we should be resilient against it. I would like 
> to see a test though.

Thanks a lot for the comment, I am new to lldb community, and got one thing a 
bit silly to ask.
Up to now, a few patches I submitted is kind of "depending on the 
compiler-generated" binary?
What am I supposed to do to **ensure the compiler generates these 
"easy-to-fault" binaries in the test?**

Like in this one, not every compiler will generate "empty ranges", and in the 
other one that is "DW_OP_div"...

I would like to add tests once I figure these out...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

In D147606#4245804 , @DavidSpickett 
wrote:

> I'm not familiar with this code, but the issue as explained I think I 
> understand.
>
> It seems like you're checking for empty ranges in two places, what does each 
> one do?
>
> Is there anything else these ranges indicate or do we think it is simply a 
> missed optimisation in the debug info producer? If they do sometimes mean 
> something, then perhaps improving the searching of the ranges is preferable 
> to discarding the empty ones.
> (though if all tests pass with this patch then these empty ranges can't be 
> that important to us can they)

Thanks for the comments, I didn't improve the search because that searching 
logic seems being used everywhere and not just restricted to filter through 
these "address ranges", I am a bit afraid of changing that also changes 
something subtle.
though it seems not making much sense to returning a "empty range" as searching 
results.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147674: Interpret ESR/FAR bits directly on watchpoint exceptions in debugserver, clarify how watchpoint descriptions in stop packets work

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: labath, JDevlieghere, omjavaid, DavidSpickett.
jasonmolenda added a project: LLDB.
Herald added subscribers: atanasyan, kristof.beyls, arichardson, sdardis.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch originally started with handling the case where a watchpoint traps 
when a write starts before the watched region, and the trap address reported 
may be outside the watched region.  I wanted to find the nearest watched region 
on these events, and relay that up to lldb so they behave correctly.  On 
reading about how the ESR and FAR registers contain details about the 
watchpoint, including possibly the watchpoint index that was triggered, I added 
code to parse those, use the watchpoint index if it's available, and pass along 
the other fields to lldb in case they become interesting.

On the lldb side, in the stop packet, watchpoints can be flagged by one of two 
ways.  One is the bog standard gdb remote serial protocol 
watch:/awatch:/rwatch: followed by a watchpoint address.  The second is an lldb 
extension to do "reason:watchpoint" followed by a "description:" whose value is 
an asciihex encoded string of three integers base10 encoded, space separated.  
These fields are an address within the watchpoint that was triggered, the 
watchpoint index, and optionally, the actual address that caused the watchpoint 
fault, which may be outside the range of any watched region.  ProcessGDBRemote 
translates watch:/awatch:/rwatch: k-v pairs into this 
reason:watchpoint+description format (with only a single address).

This third integer was added by Jaydeep Patil in 2015 ( 
https://reviews.llvm.org/D11672 ) for supporting MIPS where the hardware can 
only watch 8 byte granules, so if you watch 4 bytes and an access happens 
within that 8B granule but outside that range, execution will stop.  These 
false positive watchpoints are intended to be hidden from the user, so when we 
had a third integer that is not contained by a watchpoint region on MIPS, we 
would silently step past the watchpoint and continue without notifying the user.

In 2016 Omair Javaid ( https://reviews.llvm.org/D21516 ) started passing the 
actual trap address on Linux arm targets up as a third integer, and updated the 
conditional that turns this into a StopInfo so that the above "watchpoint hit 
outside a watched region" behavior would happen on arm.  Arm systems have a 
different behavior than the MIPS one -- if you watch 8 bytes at address 0x1008 
and someone does a 16-byte STP instruction starting at address 0x1000, 
0x1000-0x100f are modified but the watchpoint trap address reported is 0x1000.  
This address is not within the range of any watched region, so lldb can fail to 
know how to proceed.  I believe Omair was handling this, and while passing this 
value in the third argument is a good idea, enabling the same "silently step 
past this watchpoint" behavior was a mistake.  This was all took me quite a 
while to figure out, I'm not surprised it was misunderstood.  I've spent so 
much time staring at this I'm pretty sure I've got it correct now.

I documented how the three fields of this reason:watchpoint description should 
be provided.  I've kept the MIPS behavior of skipping over false positive 
watchpoints, but instead of passing an address which is outside of any 
Watchpoint range to `CreateStopReasonWithWatchpointID`, I pass a boolean 
`silently_continue` instead.  I think it is a little easier to understand what 
it's being used for.  Nothing actually did anything with the address passed to 
`CreateStopReasonWithWatchpointID` except to detect that it was not contained 
within a watchpoint region, and use this to indicate that we're doing the 
silent skipping of the watchpoint.

I wanted debugserver to use this reason:watchpoint description style reporting, 
and that dragged me into figuring out what these fields were meant to be, and 
what the "silently skip watchpoint" behavior was intended to be handling.  By 
the time I figured all the intended behaviors out, I needed to start 
documenting and, hopefully, clarifying them for when I forget again in a year.

Outside of the documentation and debugserver changes, the real changes to 
generic lldb are in `ProcessGDBRemote::SetThreadStopInfo` where I've documented 
what this is doing with the fields and, hopefully, made it a little easier to 
understand in the future.  And a minor change to 
StopInfoWatchpoint/CreateStopReasonWithWatchpointID to use a boolean flag for 
this behavior.

I added a test which has a  uint8_t[8] array and I watch one member of that, 
cast the address to a uint64_t* and write to it. I suspect this will Just Work 
on non-AArch64 targets (the watchpoint trap address will be the address of the 
uint8_t we're watching, instead of the start of the uint64_t* write I get on 
our AArch64 systems).

In debugserver, I'

[Lldb-commits] [PATCH] D147587: Fix the check in StopInfoBreakpoint for "are we running an expression"

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf79c037b6327: Fix the check in StopInfoBreakpoint for 
"are we currently running an expression" (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D147587?vs=510984&id=511237#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147587/new/

https://reviews.llvm.org/D147587

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/macosx/objc_exception_recognizer/Makefile
  lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py
  lldb/test/API/macosx/objc_exception_recognizer/main.m

Index: lldb/test/API/macosx/objc_exception_recognizer/main.m
===
--- /dev/null
+++ lldb/test/API/macosx/objc_exception_recognizer/main.m
@@ -0,0 +1,37 @@
+#import 
+
+@interface MyException : NSException
+{
+  int extra_info;
+}
+- (NSException *) initWithExtraInfo: (int) info;
+@end
+
+@implementation MyException
+- (NSException *) initWithExtraInfo: (int) info
+{
+  [super initWithName: @"NSException" reason: @"Simple Reason" userInfo: nil];
+  self->extra_info = info;
+  return self;
+}
+@end
+
+int
+main(int argc, char **argv)
+{
+  // Set a breakpoint here for plain exception:
+  @try {
+NSException *plain_exc = [[NSException alloc] initWithName: @"NSException" reason: @"Simple Reason" userInfo: nil];
+[plain_exc raise];
+  }
+  @catch (id anException) {}
+
+  // Set a breakpoint here for MyException:
+  @try {
+MyException *my_exc = [[MyException alloc] initWithExtraInfo: 100];
+[my_exc raise];
+  }
+  @catch (id anException) {}
+
+  return 0;
+}
Index: lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py
===
--- /dev/null
+++ lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py
@@ -0,0 +1,69 @@
+"""
+Test that the built in ObjC exception throw recognizer works
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestObjCRecognizer(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessDarwin
+def test_exception_recognizer_sub_class(self):
+"""There can be many tests in a test case - describe this test here."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.m")
+self.objc_recognizer_test(True)
+
+@skipUnlessDarwin
+def test_exception_recognizer_plain(self):
+"""There can be many tests in a test case - describe this test here."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.m")
+self.objc_recognizer_test(False)
+
+def objc_recognizer_test(self, sub_class):
+"""Make sure we stop at the exception and get all the fields out of the recognizer.
+   If sub_class is True, we make a subclass of NSException and throw that."""
+if sub_class:
+bkpt_string = "Set a breakpoint here for MyException"
+else:
+bkpt_string = "Set a breakpoint here for plain exception"
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   bkpt_string, self.main_source_file)
+
+# Now turn on the ObjC Exception breakpoint and continue to hit it:
+exception_bkpt = target.BreakpointCreateForException(lldb.eLanguageTypeObjC, False, True)
+self.assertTrue(exception_bkpt.GetNumLocations() > 0, "Got some exception locations")
+
+threads = lldbutil.continue_to_breakpoint(process, exception_bkpt)
+self.assertEqual(len(threads), 1, "One thread hit exception breakpoint")
+frame = threads[0].frame[0]
+
+var_opts = lldb.SBVariablesOptions()
+var_opts.SetIncludeRecognizedArguments(True)
+var_opts.SetUseDynamic(True)
+vars = frame.GetVariables(var_opts)
+self.assertEqual(len(vars), 1, "Got the synthetic argument")
+self.assertTrue(vars[0].IsValid(), "Got a valid Exception variable")
+
+# This will be a pointer
+
+ns_exception_children = [ValueCheck(type="NSObject"),
+ ValueCheck(name="name", summary='"NSException"'),
+ ValueCheck(name="reason", summary='"Simple Reason"'),
+ ValueCheck(name="userInfo"),
+ ValueCheck(name="reserved")]
+ns_exception = ValueCheck(type="NSException", children=ns_exception_children) 
+if not sub_class:
+simple_check = ValueCheck(name="exception", dereference=ns_exception)
+simple_check.check_value(self, vars[0], "Simple exception is right")
+else:
+my_exception_children = [ns_exception, ValueCheck(name="e

[Lldb-commits] [lldb] f79c037 - Fix the check in StopInfoBreakpoint for "are we currently running an expression"

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-04-05T17:14:25-07:00
New Revision: f79c037b63278bc5b4481a1a55c68e42f0ea1461

URL: 
https://github.com/llvm/llvm-project/commit/f79c037b63278bc5b4481a1a55c68e42f0ea1461
DIFF: 
https://github.com/llvm/llvm-project/commit/f79c037b63278bc5b4481a1a55c68e42f0ea1461.diff

LOG: Fix the check in StopInfoBreakpoint for "are we currently running an 
expression"

We were checking "WasTheLastResumeForUserExpression" but that returns true even
if that expression was completed, provided we haven't run again.  This uses a
better check.

This is actually fairly hard to trigger.  It happens the first time you hit an
objc_exception_throw breakpoint and invoke that frame recognizer for that.  But
I couldn't trigger it using a Python based frame recognizer.  So I wrote a test
for the objc_exception_throw_breakpoint recognizer which should have been there
anyway...  It fails (the target auto-continues) w/o this patch and succeeds with
it.

Differential Revision: https://reviews.llvm.org/D147587

Added: 
lldb/test/API/macosx/objc_exception_recognizer/Makefile
lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py
lldb/test/API/macosx/objc_exception_recognizer/main.m

Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Target/StopInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 3ffacb52299b9..1857dd0a7ada9 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -284,6 +284,13 @@ class ProcessModID {
 return m_resume_id == m_last_user_expression_resume;
   }
 
+  bool IsRunningExpression() const {
+// Don't return true if we are no longer running an expression:
+if (m_running_user_expression || m_running_utility_function)
+  return true;
+return false;
+  }
+
   void SetRunningUserExpression(bool on) {
 if (on)
   m_running_user_expression++;

diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index a98fc28c7338d..9fd0e0a5674ec 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -332,7 +332,7 @@ class StopInfoBreakpoint : public StopInfo {
 
   ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
   Process *process = exe_ctx.GetProcessPtr();
-  if (process->GetModIDRef().IsLastResumeForUserExpression()) {
+  if (process->GetModIDRef().IsRunningExpression()) {
 // If we are in the middle of evaluating an expression, don't run
 // asynchronous breakpoint commands or expressions.  That could
 // lead to infinite recursion if the command or condition re-calls

diff  --git a/lldb/test/API/macosx/objc_exception_recognizer/Makefile 
b/lldb/test/API/macosx/objc_exception_recognizer/Makefile
new file mode 100644
index 0..66119b1b48248
--- /dev/null
+++ b/lldb/test/API/macosx/objc_exception_recognizer/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -framework Foundation
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py 
b/lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py
new file mode 100644
index 0..d7e5eb310a191
--- /dev/null
+++ b/lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py
@@ -0,0 +1,69 @@
+"""
+Test that the built in ObjC exception throw recognizer works
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestObjCRecognizer(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessDarwin
+def test_exception_recognizer_sub_class(self):
+"""There can be many tests in a test case - describe this test here."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.m")
+self.objc_recognizer_test(True)
+
+@skipUnlessDarwin
+def test_exception_recognizer_plain(self):
+"""There can be many tests in a test case - describe this test here."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.m")
+self.objc_recognizer_test(False)
+
+def objc_recognizer_test(self, sub_class):
+"""Make sure we stop at the exception and get all the fields out of 
the recognizer.
+   If sub_class is True, we make a subclass of NSException and throw 
that."""
+if sub_class:
+bkpt_string = "Set a breakpoint here for MyException"
+else:
+bkpt_string = "Set a breakpoint here for plain exception"
+
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+   bkpt_string, self.main_source_file)
+
+# Now turn on the ObjC Exception breakpoint and continue to hit it

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get export 
"
"table entry name: {0}",
llvm::fmt_consume(std::move(err)));
+  llvm::consumeError(std::move(err));

compnerd wrote:
> bulbazord wrote:
> > If logging is enabled, we are moving from the same object twice, no?
> > 
> > I think we should rethink the LLDB_LOG macro when it comes to errors :/
> Yeah ... I was worried about that.
I should mention that at least on MSVC I _can_ get away with it:

```
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found 
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
 ObjectFilePECOFF::AppendFromExportTable - failed to get export table entry 
name: RVA 0x0 for export ordinal table not found
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147669/new/

https://reviews.llvm.org/D147669

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get export 
"
"table entry name: {0}",
llvm::fmt_consume(std::move(err)));
+  llvm::consumeError(std::move(err));

bulbazord wrote:
> If logging is enabled, we are moving from the same object twice, no?
> 
> I think we should rethink the LLDB_LOG macro when it comes to errors :/
Yeah ... I was worried about that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147669/new/

https://reviews.llvm.org/D147669

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get export 
"
"table entry name: {0}",
llvm::fmt_consume(std::move(err)));
+  llvm::consumeError(std::move(err));

If logging is enabled, we are moving from the same object twice, no?

I think we should rethink the LLDB_LOG macro when it comes to errors :/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147669/new/

https://reviews.llvm.org/D147669

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd created this revision.
compnerd added reviewers: bulbazord, sgraenitz, labath.
compnerd added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
compnerd requested review of this revision.

Ensure that we explicitly indicate that we would like the move semantics
in the assignment.  With MSVC 14.35.32215, the assignment would not
trigger a NVRO and copy constructor would be invoked, which resulted in
a check failure under `LLVM_ENABLE_ABI_BREAKING_CHANGES`.  The error
path is meant to log and continue on the failure but would instead abort
when built with assertions.

Secondary to the assignment cast check, we would not ensure that the
error is consumed in the case that logging is disabled.  Ensure that we
properly drop the error on the floor or we would re-trigger the checked
failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147669

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -865,11 +865,12 @@
   // Read each export table entry, ordered by ordinal instead of by name.
   for (const auto &entry : m_binary->export_directories()) {
 llvm::StringRef sym_name;
-if (auto err = entry.getSymbolName(sym_name)) {
+if (auto err = std::move(entry.getSymbolName(sym_name))) {
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get export 
"
"table entry name: {0}",
llvm::fmt_consume(std::move(err)));
+  llvm::consumeError(std::move(err));
   continue;
 }
 Symbol symbol;
@@ -886,11 +887,12 @@
   // Forwarder exports are redirected by the loader transparently, but keep
   // it in symtab and make a note using the symbol name.
   llvm::StringRef forwarder_name;
-  if (auto err = entry.getForwardTo(forwarder_name)) {
+  if (auto err = std::move(entry.getForwardTo(forwarder_name))) {
 LLDB_LOG(log,
  "ObjectFilePECOFF::AppendFromExportTable - failed to get "
  "forwarder name of forwarder export '{0}': {1}",
  sym_name, llvm::fmt_consume(std::move(err)));
+llvm::consumeError(std::move(err));
 continue;
   }
   llvm::SmallString<256> new_name = 
{symbol.GetDisplayName().GetStringRef(),
@@ -901,11 +903,12 @@
 }
 
 uint32_t function_rva;
-if (auto err = entry.getExportRVA(function_rva)) {
+if (auto err = std::move(entry.getExportRVA(function_rva))) {
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get "
"address of export entry '{0}': {1}",
sym_name, llvm::fmt_consume(std::move(err)));
+  llvm::consumeError(std::move(err));
   continue;
 }
 // Skip the symbol if it doesn't look valid.


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -865,11 +865,12 @@
   // Read each export table entry, ordered by ordinal instead of by name.
   for (const auto &entry : m_binary->export_directories()) {
 llvm::StringRef sym_name;
-if (auto err = entry.getSymbolName(sym_name)) {
+if (auto err = std::move(entry.getSymbolName(sym_name))) {
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get export "
"table entry name: {0}",
llvm::fmt_consume(std::move(err)));
+  llvm::consumeError(std::move(err));
   continue;
 }
 Symbol symbol;
@@ -886,11 +887,12 @@
   // Forwarder exports are redirected by the loader transparently, but keep
   // it in symtab and make a note using the symbol name.
   llvm::StringRef forwarder_name;
-  if (auto err = entry.getForwardTo(forwarder_name)) {
+  if (auto err = std::move(entry.getForwardTo(forwarder_name))) {
 LLDB_LOG(log,
  "ObjectFilePECOFF::AppendFromExportTable - failed to get "
  "forwarder name of forwarder export '{0}': {1}",
  sym_name, llvm::fmt_consume(std::move(err)));
+llvm::consumeError(std::move(err));
 continue;
   }
   llvm::SmallString<256> new_name = {symbol.GetDisplayName().GetStringRef(),
@@ -901,11 +903,12 @@
 }
 
 uint32_t function_rva;
-if (auto err = entry.getExportRVA(function_rva)) {
+if (auto err = std::move(entry.getExportRVA(function_rva))) {
   LLDB_LOG(log,
"ObjectFilePECOFF::AppendFromExportTable - failed to get "
"address of export entry '{0}': {1}",
sym_name

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

The change looks fine and regardless of whether this makes sense or even 
complies with the standard, we should be resilient against it. I would like to 
see a test though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e289b53 - [lldb] Remove unused functions from RegisterContextLinux_x86

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-04-05T20:16:48+02:00
New Revision: e289b53f4d9bffd71613d6f91747bf9bda0ae352

URL: 
https://github.com/llvm/llvm-project/commit/e289b53f4d9bffd71613d6f91747bf9bda0ae352
DIFF: 
https://github.com/llvm/llvm-project/commit/e289b53f4d9bffd71613d6f91747bf9bda0ae352.diff

LOG: [lldb] Remove unused functions from RegisterContextLinux_x86

These should be overridden by individual subclasses, they ended up here
because of copy-pasta.

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h
index 663c1d9d123d5..0e1863864aa6c 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h
@@ -19,15 +19,6 @@ class RegisterContextLinux_x86 : public 
RegisterInfoInterface {
RegisterInfo orig_ax_info)
   : RegisterInfoInterface(target_arch), m_orig_ax_info(orig_ax_info) {}
 
-  static size_t GetGPRSizeStatic();
-  size_t GetGPRSize() const override { return GetGPRSizeStatic(); }
-
-  const lldb_private::RegisterInfo *GetRegisterInfo() const override;
-
-  uint32_t GetRegisterCount() const override;
-
-  uint32_t GetUserRegisterCount() const override;
-
   const RegisterInfo &GetOrigAxInfo() const { return m_orig_ax_info; }
 
 private:



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz updated this revision to Diff 511155.
sgraenitz added a comment.

Second try for the out-of-range test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147642/new/

https://reviews.llvm.org/D147642

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml

Index: lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
@@ -0,0 +1,62 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file -contents %t | FileCheck %s
+
+## Test that R_ARM_ABS32 relocations are resolved in .debug_info sections on aarch32.
+## REL-type relocations store implicit addend as signed values inline.
+## We relocate the symbol foo with 4 different addends and bar once in the .debug_info section.
+## Results that exceed the 32-bit range or overflow are logged and ignored.
+
+# CHECK:  Name: .debug_info
+# CHECK:  Data:  (
+#
+#  Addends: Zero Positive Negative Overflow Out-of-range
+#    04030201 D6FF D5FF FF7F
+# CHECK-NEXT: : 2A00 2E030201  D5FF FF7F
+# CHECK-NEXT: )
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:.debug_info
+Type:SHT_PROGBITS
+Content: 04030201D6FFD57F
+  - Name:.rel.debug_info
+Type:SHT_REL
+Info:.debug_info
+Relocations:
+  - Offset:  0x0
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x4
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x8
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0xC
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x10
+Symbol:  bar
+Type:R_ARM_ABS32
+Symbols:
+  - Name:.debug_info
+Type:STT_SECTION
+Section: .debug_info
+  - Name:foo
+Type:STT_FUNC
+Section: .debug_info
+Value:   0x002A
+  - Name:bar
+Type:STT_FUNC
+Section: .debug_info
+Value:   0xFF00
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2637,6 +2637,43 @@
   }
 }
 
+static void ApplyELF32ABS32Relocation(Symtab *symtab, ELFRelocation &rel,
+  DataExtractor &debug_data,
+  Section *rel_section) {
+  Log *log = GetLog(LLDBLog::Modules);
+  Symbol *symbol = symtab->FindSymbolByID(ELFRelocation::RelocSymbol32(rel));
+  if (symbol) {
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (value == LLDB_INVALID_ADDRESS) {
+  const char *name = symbol->GetName().GetCString();
+  LLDB_LOGF(log, "Debug info symbol invalid: %s", name);
+  return;
+}
+assert(llvm::isUInt<32>(value) && "Valid addresses are 32-bit");
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint8_t *dst = data_buffer->GetBytes() + rel_section->GetFileOffset() +
+   ELFRelocation::RelocOffset32(rel);
+// Implicit addend is stored inline as a signed value.
+int32_t addend = *reinterpret_cast(dst);
+// The sum must be positive. This extra check prevents UB from overflow in
+// the actual range check below.
+if (addend < 0 && static_cast(std::abs(addend)) > value) {
+  LLDB_LOGF(log, "Debug info relocation overflow: 0x%" PRIx64,
+static_cast(value) + addend);
+  return;
+}
+if (!llvm::isUInt<32>(value + addend)) {
+  LLDB_LOGF(log, "Debug info relocation out of range: 0x%" PRIx64, value);
+  return;
+}
+uint32_t addr = value + addend;
+memcpy(dst, &addr, sizeof(uint32_t));
+  }
+}
+
 unsigned ObjectFileELF::ApplyRelocations(
 Symtab *symtab, const ELFHeader *hdr, const ELFSectionHeader *rel_hdr,
 const ELFSectionHeader *symtab_hdr, const ELFSectionHeader *debug_hdr,
@@ -2667,6 +2704,15 @@
 
 if (hdr->Is32Bit()) {
   switch (hdr->e_machine) {
+  case llvm::ELF::EM_ARM:
+switc

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz updated this revision to Diff 511151.
sgraenitz added a comment.

Add the missing 5th relocation to the test that exercises the out-of-range case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147642/new/

https://reviews.llvm.org/D147642

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml

Index: lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
@@ -0,0 +1,58 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file -contents %t | FileCheck %s
+
+## Test that R_ARM_ABS32 relocations are resolved in .debug_info sections on aarch32.
+## We relocate the symbol foo with 5 different addends in the .debug_info section.
+## REL-type relocations store implicit addend as signed values inline.
+## Results that exceed the 32-bit range or overflow are logged and ignored.
+
+# CHECK:  Name: .debug_info
+# CHECK:  Data:  (
+#
+#  Addends: Zero Positive Negative Overflow Out-of-range
+#    04030201 D6FF D5FF FF7F
+# CHECK-NEXT: : 2A00 2E030201  D5FF FF7F
+# CHECK-NEXT: )
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:.debug_info
+Type:SHT_PROGBITS
+Content: 04030201D6FFD57F
+  - Name:.rel.debug_info
+Type:SHT_REL
+Info:.debug_info
+Relocations:
+  - Offset:  0x0
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x4
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x8
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0xC
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x10
+Symbol:  foo
+Type:R_ARM_ABS32
+Symbols:
+  - Name:.debug_info
+Type:STT_SECTION
+Section: .debug_info
+  - Name:foo
+Type:STT_FUNC
+Section: .debug_info
+Value:   0x002A
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2637,6 +2637,43 @@
   }
 }
 
+static void ApplyELF32ABS32Relocation(Symtab *symtab, ELFRelocation &rel,
+  DataExtractor &debug_data,
+  Section *rel_section) {
+  Log *log = GetLog(LLDBLog::Modules);
+  Symbol *symbol = symtab->FindSymbolByID(ELFRelocation::RelocSymbol32(rel));
+  if (symbol) {
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (value == LLDB_INVALID_ADDRESS) {
+  const char *name = symbol->GetName().GetCString();
+  LLDB_LOGF(log, "Debug info symbol invalid: %s", name);
+  return;
+}
+assert(llvm::isUInt<32>(value) && "Valid addresses are 32-bit");
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint8_t *dst = data_buffer->GetBytes() + rel_section->GetFileOffset() +
+   ELFRelocation::RelocOffset32(rel);
+// Implicit addend is stored inline as a signed value.
+int32_t addend = *reinterpret_cast(dst);
+// The sum must be positive. This extra check prevents UB from overflow in
+// the actual range check below.
+if (addend < 0 && static_cast(std::abs(addend)) > value) {
+  LLDB_LOGF(log, "Debug info relocation overflow: 0x%" PRIx64,
+static_cast(value) + addend);
+  return;
+}
+if (!llvm::isUInt<32>(value + addend)) {
+  LLDB_LOGF(log, "Debug info relocation out of range: 0x%" PRIx64, value);
+  return;
+}
+uint32_t addr = value + addend;
+memcpy(dst, &addr, sizeof(uint32_t));
+  }
+}
+
 unsigned ObjectFileELF::ApplyRelocations(
 Symtab *symtab, const ELFHeader *hdr, const ELFSectionHeader *rel_hdr,
 const ELFSectionHeader *symtab_hdr, const ELFSectionHeader *debug_hdr,
@@ -2667,6 +2704,15 @@
 
 if (hdr->Is32Bit()) {
   switch (hdr->e_machine) {
+  case llvm::ELF::EM_ARM:
+switch (reloc_type(rel)) {
+case R_ARM_ABS32:
+  ApplyELF32ABS32Relocation(symtab, 

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D145580#4245911 , @DavidSpickett 
wrote:

> @bulbazord Please take a look and see if I've done the plugin/core code split 
> correctly.

This looks fine. Thanks for being flexible and working with us to find 
something that maintains the split even if the end result feels more complex.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Adding an abstraction for reading implicit addend or somehow integrate it into 
`ELFRelocation::RelocAddend32()` might be more confusing for the reader than 
keeping it in the function inline. What do you think?




Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2660
+// Implicit addend is stored inline as a signed value.
+int32_t addend = *reinterpret_cast(dst);
+// The sum must be positive. This extra check prevents UB from overflow in

IIUC we'd want to account for an endianness difference between debugger and 
target (in theory). However, non of the other cases seems to do it, so I didn't 
start with it either.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2711
+  ApplyELF32ABS32Relocation(symtab, rel, debug_data, rel_section);
+  break;
+default:

In think debug info relocations have been `R_ARM_ABS32` in all  ARM/Thumb, 
pic/non-pic variations I tried. @peter.smith Does that hold in general? 
Otherwise, I'd like to report errors for the other cases and not let them run 
into the assert(false) below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147642/new/

https://reviews.llvm.org/D147642

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147627: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

This would be good to land in preparation for D147642 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147627/new/

https://reviews.llvm.org/D147627

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added reviewers: DavidSpickett, peter.smith, labath, davide.
Herald added subscribers: omjavaid, kristof.beyls, emaste.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLDB.

Allow the ObjectFileELF plugin to resolve R_ARM_ABS32 relocations from AArch32 
object files. This fixes https://github.com/llvm/llvm-project/issues/61948

The existing architectures work with RELA-type relocation records that read 
addend from the relocation entry. REL-type relocations in AArch32 store addend 
in-place.
The new function doesn't re-use ELFRelocation::RelocAddend32(), because the 
interface doesn't match: in addition to the relocation entry we need the actual 
target section memory.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147642

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml

Index: lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
@@ -0,0 +1,55 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file -contents %t | FileCheck %s
+
+## Test that R_ARM_ABS32 relocations are resolved in .debug_info sections on aarch32.
+## We relocate the symbol foo with 4 different addends in the .debug_info section.
+## REL-type relocations store implicit addend as signed values inline.
+## Results that exceed the 32-bit range or overflow are logged and ignored.
+
+# CHECK:  Name: .debug_info
+# CHECK:  Data:  (
+#
+#  Addends: Zero Positive Negative Overflow Out-of-range
+#    04030201 D6FF D5FF FF7F
+# CHECK-NEXT: : 2A00 2E030201  D5FF FF7F
+# CHECK-NEXT: )
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:.debug_info
+Type:SHT_PROGBITS
+Content: 04030201D6FFD57F
+  - Name:.rel.debug_info
+Type:SHT_REL
+Info:.debug_info
+Relocations:
+  - Offset:  0x0
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x4
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x8
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0xC
+Symbol:  foo
+Type:R_ARM_ABS32
+Symbols:
+  - Name:.debug_info
+Type:STT_SECTION
+Section: .debug_info
+  - Name:foo
+Type:STT_FUNC
+Section: .debug_info
+Value:   0x002A
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2637,6 +2637,43 @@
   }
 }
 
+static void ApplyELF32ABS32Relocation(Symtab *symtab, ELFRelocation &rel,
+  DataExtractor &debug_data,
+  Section *rel_section) {
+  Log *log = GetLog(LLDBLog::Modules);
+  Symbol *symbol = symtab->FindSymbolByID(ELFRelocation::RelocSymbol32(rel));
+  if (symbol) {
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (value == LLDB_INVALID_ADDRESS) {
+  const char *name = symbol->GetName().GetCString();
+  LLDB_LOGF(log, "Debug info symbol invalid: %s", name);
+  return;
+}
+assert(llvm::isUInt<32>(value) && "Valid addresses are 32-bit");
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint8_t *dst = data_buffer->GetBytes() + rel_section->GetFileOffset() +
+   ELFRelocation::RelocOffset32(rel);
+// Implicit addend is stored inline as a signed value.
+int32_t addend = *reinterpret_cast(dst);
+// The sum must be positive. This extra check prevents UB from overflow in
+// the actual range check below.
+if (addend < 0 && static_cast(std::abs(addend)) > value) {
+  LLDB_LOGF(log, "Debug info relocation overflow: 0x%" PRIx64,
+static_cast(value) + addend);
+  return;
+}
+if (!llvm::isUInt<32>(value + addend)) {
+  LLDB_LOGF(log, "Debug info relocation out of range: 0x%" PRIx64, value);
+  return;
+}
+uint32_t addr = va

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:127
  llvm::Triple::hexagon, llvm::Triple::mips, llvm::Triple::mips64el,
- llvm::Triple::mipsel, llvm::Triple::systemz},
+ llvm::Triple::mipsel, llvm::Triple::msp430, llvm::Triple::systemz},
 llvm::Triple::Linux);

DavidSpickett wrote:
> If this is bare metal why do we have changes to Linux code here? Or is this 
> the default platform used when connecting to the msp430 debug stub.
> 
> That said, I'm not sure we A: support Hexagon or B: it runs linux either :)
Hexagon is definitely supported AFAIK but I'm not sure it runs Linux. It does 
looks like we support an SysV ABI class for Hexagon though, so perhaps that's 
somewhat indicative that it runs something with that ABI...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146965/new/

https://reviews.llvm.org/D146965

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147587: Fix the check in StopInfoBreakpoint for "are we running an expression"

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

LGTM. It'd be nice if we could have a test that wasn't specific to Darwin or 
ObjC though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147587/new/

https://reviews.llvm.org/D147587

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147627: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added reviewers: DavidSpickett, SixWeining.
Herald added a subscriber: emaste.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLDB.

ObjectFileELF::ApplyRelocations() considered all 32-bit input objects to be 
i386 and didn't provide good error messages for AArch32 objects. Please find an 
example in https://github.com/llvm/llvm-project/issues/61948
While we are here, let' improve the situation for unsupported architectures as 
well. I think we should report the error here too and not silently fail (or 
crash with assertions enabled).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147627

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,48 @@
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
-  DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
-  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
-  WritableDataBuffer *data_buffer =
-  llvm::cast(data_buffer_sp.get());
-  uint32_t *dst = reinterpret_cast(
-  data_buffer->GetBytes() + f_offset);
-
-  addr_t value = symbol->GetAddressRef().GetFileAddress();
-  if (rel.IsRela()) {
-value += ELFRelocation::RelocAddend32(rel);
+  switch (hdr->e_machine) {
+  case llvm::ELF::EM_386:
+switch (reloc_type(rel)) {
+case R_386_32:
+  symbol = symtab->FindSymbolByID(reloc_symbol(rel));
+  if (symbol) {
+addr_t f_offset =
+rel_section->GetFileOffset() + 
ELFRelocation::RelocOffset32(rel);
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint32_t *dst = reinterpret_cast(
+data_buffer->GetBytes() + f_offset);
+
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (rel.IsRela()) {
+  value += ELFRelocation::RelocAddend32(rel);
+} else {
+  value += *dst;
+}
+*dst = value;
   } else {
-value += *dst;
+GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+rel_section->GetName().AsCString(), i,
+reloc_symbol(rel));
   }
-  *dst = value;
-} else {
-  GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+  break;
+case R_386_PC32:
+  GetModule()->ReportError("unsupported i386 relocation:"
+   " .rel{0}[{1}], type {2}",
rel_section->GetName().AsCString(), i,
-   reloc_symbol(rel));
+   reloc_type(rel));
+  break;
+default:
+  assert(false && "unexpected relocation type");
+  break;
 }
 break;
-  case R_386_PC32:
   default:
-GetModule()->ReportError("unsupported 32-bit relocation:"
- " .rel{0}[{1}], type {2}",
- rel_section->GetName().AsCString(), i,
- reloc_type(rel));
+GetModule()->ReportError("unsupported 32-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 } else {
   switch (hdr->e_machine) {
@@ -2743,7 +2753,8 @@
 }
 break;
   default:
-assert(false && "unsupported machine");
+GetModule()->ReportError("unsupported 64-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 }
   }


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,48 @@
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(r

[Lldb-commits] [lldb] 7e28a2c - Skip tests under asan

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2023-04-05T09:00:55-07:00
New Revision: 7e28a2c9f49d74b0dde02fd81b40f837f6140083

URL: 
https://github.com/llvm/llvm-project/commit/7e28a2c9f49d74b0dde02fd81b40f837f6140083
DIFF: 
https://github.com/llvm/llvm-project/commit/7e28a2c9f49d74b0dde02fd81b40f837f6140083.diff

LOG: Skip tests under asan

Added: 


Modified: 
lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
lldb/test/API/macosx/universal64/TestUniversal64.py

Removed: 




diff  --git a/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py 
b/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
index afd036524bfe5..f9bf008d9dc43 100644
--- a/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
+++ b/lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py
@@ -8,6 +8,8 @@
 
 class TestFullLtoStepping(TestBase):
 
+# The Makefile manually invokes clang.
+@skipIfAsan
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=['<', '13.0'])
 @skipUnlessDarwin

diff  --git a/lldb/test/API/macosx/universal64/TestUniversal64.py 
b/lldb/test/API/macosx/universal64/TestUniversal64.py
index 80292c04dd123..2296ac8b8324c 100644
--- a/lldb/test/API/macosx/universal64/TestUniversal64.py
+++ b/lldb/test/API/macosx/universal64/TestUniversal64.py
@@ -16,6 +16,8 @@ def do_test(self):
 # The dynamic loader doesn't support fat64 executables so we can't
 # actually launch them here.
 
+# The Makefile manually invokes clang.
+@skipIfAsan
 @skipUnlessDarwin
 @skipIfDarwinEmbedded
 @skipIf(macos_version=["<", "11.0"])
@@ -24,6 +26,8 @@ def test_universal64_executable(self):
 self.build(debug_info="dsym")
 self.do_test()
 
+# The Makefile manually invokes clang.
+@skipIfAsan
 @skipUnlessDarwin
 @skipIfDarwinEmbedded
 @skipIf(macos_version=["<", "11.0"])



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 9 inline comments as done.
DavidSpickett added a comment.

@bulbazord Please take a look and see if I've done the plugin/core code split 
correctly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 511096.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

Move the type generation into a new plugin type, RegisterTypeBuilder.

This means we're still resuing types if you read the same register
multiple times, but we are recreating the plugin each time. So that
could be improved if people think it's worth it.

The code path is now:

- register read calls dumpregister
- which calls GetRegisterType on Target
- Target gets the plugin and asks it to make the type

The way it does all that is exactly the same as before, but now
we aren't linking directly to a plugin to do it.

Also I am assuming that RegisterTypeBuilderClang is the only implementation
and is always present (a singleton plugin sort of). Which seems fine but I
don't know if that's a good way to treat plugins in general.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/include/lldb/Target/RegisterTypeBuilder.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
  lldb/unittests/Target/RegisterFlagsTest.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -239,6 +239,12 @@
 * LLDB is now able to show the subtype of signals found in a core file. For example
   memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
 
+* LLDB can now display register fields if they are described in target XML sent
+  by a debug server such as ``gdbserver`` (``lldb-server`` does not currently produce
+  this information). Fields are only printed when reading named registers, for
+  example ``register read cpsr``. They are not shown when reading a register set,
+  ``register read -s 0``.
+
 Changes to Sanitizers
 -
 * For Darwin users that override weak symbols, note that the dynamic linker will
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,3 +121,19 @@
 make_field(20, 21), make_field(12, 19), make_field(8, 11),
 make_field(0, 7)});
 }
+
+TEST(RegisterFieldsTest, ReverseFieldOrder) {
+  // Unchanged
+  RegisterFlags rf("", 4, {make_field(0, 31)});
+  ASSERT_EQ(0x12345678ULL, rf.ReverseFieldOrder(0x12345678));
+
+  // Swap the two halves around.
+  RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)});
+  ASSERT_EQ(0x56781234ULL, rf2.ReverseFieldOrder(0x12345678));
+
+  // Many small fields.
+  RegisterFlags rf3("", 4,
+{make_field(31, 31), make_field(30, 30), make_field(29, 29),
+ make_field(28, 28)});
+  ASSERT_EQ(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" Check that register fields found in target XML are properly processed.
+
+These tests make XML out of string substitution. This can lead to some strange
+failures. Check that the final XML is valid and each child is indented more than
+the parent tag.
+"""
+
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+class MultiDocResponder(MockGDBServerResponder):
+# docs is a dictionary of filename -> file content.
+def __init__(self, docs):
+super().__init__()
+self.docs = docs
+
+def qXferRead(self, obj, annex, offset, length):
+try:
+return self.docs[annex], False
+except KeyError:
+return None,
+
+def readRegister(self, regnum):
+return "E01"
+
+def readRegiste

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I'm not familiar with this code, but the issue as explained I think I 
understand.

It seems like you're checking for empty ranges in two places, what does each 
one do?

Is there anything else these ranges indicate or do we think it is simply a 
missed optimisation in the debug info producer? If they do sometimes mean 
something, then perhaps improving the searching of the ranges is preferable to 
discarding the empty ones.
(though if all tests pass with this patch then these empty ranges can't be that 
important to us can they)




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1318
+  if (range_base >= subprogram_low_pc) {
+if (range.IsValid())
+  block->AddRange(Block::Range(range_base - subprogram_low_pc,

Combine this into one if. Also I'd put the Valid check first, just seems like 
the right order for it (not that it matters really).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy created this revision.
jwnhy added reviewers: Michael137, DavidSpickett, JDevlieghere.
Herald added a project: All.
jwnhy requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch resolves an issue that lldb not be able to
match the correct range of certain address.

This issue caused by other compilers like gcc generates
"empty ranges" like [address, address) in the DIE. These
"empty ranges" cause lldb matches address with these
ranges unintentionally and causes unpredictable result.
(In #61942, a variable dispearred due to this issue)

This patch resolves this issue by discarding these "empty
ranges" during the parsing of debugging information.

Another approach in fixing this might be changing the
logic of "FindEntryThatContains" and let it try harder
if met "empty ranges".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147606

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1314,10 +1314,11 @@
 for (size_t i = 0; i < num_ranges; ++i) {
   const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
   const addr_t range_base = range.GetRangeBase();
-  if (range_base >= subprogram_low_pc)
-block->AddRange(Block::Range(range_base - subprogram_low_pc,
+  if (range_base >= subprogram_low_pc) {
+if (range.IsValid())
+  block->AddRange(Block::Range(range_base - subprogram_low_pc,
  range.GetByteSize()));
-  else {
+  } else {
 GetObjectFile()->GetModule()->ReportError(
 "{0:x8}: adding range [{1:x16}-{2:x16}) which has a base "
 "that is less than the function's low PC {3:x16}. Please file "
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1065,8 +1065,9 @@
 
   DWARFRangeList ranges;
   for (const llvm::DWARFAddressRange &llvm_range : *llvm_ranges) {
-ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
-llvm_range.HighPC - llvm_range.LowPC));
+if (llvm_range.HighPC - llvm_range.LowPC > 0)
+  ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
+  llvm_range.HighPC - 
llvm_range.LowPC));
   }
   return ranges;
 }


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1314,10 +1314,11 @@
 for (size_t i = 0; i < num_ranges; ++i) {
   const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
   const addr_t range_base = range.GetRangeBase();
-  if (range_base >= subprogram_low_pc)
-block->AddRange(Block::Range(range_base - subprogram_low_pc,
+  if (range_base >= subprogram_low_pc) {
+if (range.IsValid())
+  block->AddRange(Block::Range(range_base - subprogram_low_pc,
  range.GetByteSize()));
-  else {
+  } else {
 GetObjectFile()->GetModule()->ReportError(
 "{0:x8}: adding range [{1:x16}-{2:x16}) which has a base "
 "that is less than the function's low PC {3:x16}. Please file "
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1065,8 +1065,9 @@
 
   DWARFRangeList ranges;
   for (const llvm::DWARFAddressRange &llvm_range : *llvm_ranges) {
-ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
-llvm_range.HighPC - llvm_range.LowPC));
+if (llvm_range.HighPC - llvm_range.LowPC > 0)
+  ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
+  llvm_range.HighPC - llvm_range.LowPC));
   }
   return ranges;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e42f920 - [lldb] Remove unused private field 'm_orig_rax_info' in RegisterContextLinux_x86_64.h (NFC)

[Jie Fu via lldb-commits]

Author: Jie Fu
Date: 2023-04-05T19:54:23+08:00
New Revision: e42f920918ca5b84329970825ce6a69b53f17bdb

URL: 
https://github.com/llvm/llvm-project/commit/e42f920918ca5b84329970825ce6a69b53f17bdb
DIFF: 
https://github.com/llvm/llvm-project/commit/e42f920918ca5b84329970825ce6a69b53f17bdb.diff

LOG: [lldb] Remove unused private field 'm_orig_rax_info' in 
RegisterContextLinux_x86_64.h (NFC)

/data/llvm-project/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h:32:30:
 error: private field 'm_orig_rax_info' is not used 
[-Werror,-Wunused-private-field]
  lldb_private::RegisterInfo m_orig_rax_info;
 ^
1 error generated.

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h 
b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
index 2918e5d5ec31..d141ba66b4e2 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
@@ -29,7 +29,6 @@ class RegisterContextLinux_x86_64
   const lldb_private::RegisterInfo *m_register_info_p;
   uint32_t m_register_info_count;
   uint32_t m_user_register_count;
-  lldb_private::RegisterInfo m_orig_rax_info;
 };
 
 #endif



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147045: [lldb] Drop RegisterInfoInterface::GetDynamicRegisterInfo

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG933d3ee60007: [lldb] Drop 
RegisterInfoInterface::GetDynamicRegisterInfo (authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147045/new/

https://reviews.llvm.org/D147045

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_s390x.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_s390x.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
@@ -41,26 +41,6 @@
 return m_target_arch;
   }
 
-  virtual const lldb_private::RegisterInfo *
-  GetDynamicRegisterInfo(const char *reg_name) const {
-const std::vector *d_register_infos =
-GetDynamicRegisterInfoP();
-if (d_register_infos != nullptr) {
-  std::vector::const_iterator pos =
-  d_register_infos->begin();
-  for (; pos < d_register_infos->end(); pos++) {
-if (::strcmp(reg_name, pos->name) == 0)
-  return (d_register_infos->data() + (pos - d_register_infos->begin()));
-  }
-}
-return nullptr;
-  }
-
-  virtual const std::vector *
-  GetDynamicRegisterInfoP() const {
-return nullptr;
-  }
-
 private:
   lldb_private::ArchSpec m_target_arch;
 };
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
@@ -9,9 +9,10 @@
 #ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERCONTEXTLINUX_X86_64_H
 #define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERCONTEXTLINUX_X86_64_H
 
-#include "RegisterInfoInterface.h"
+#include "Plugins/Process/Utility/RegisterContextLinux_x86.h"
 
-class RegisterContextLinux_x86_64 : public lldb_private::RegisterInfoInterface {
+class RegisterContextLinux_x86_64
+: public lldb_private::RegisterContextLinux_x86 {
 public:
   RegisterContextLinux_x86_64(const lldb_private::ArchSpec &target_arch);
 
@@ -24,14 +25,11 @@
 
   uint32_t GetUserRegisterCount() const override;
 
-  const std::vector *
-  GetDynamicRegisterInfoP() const override;
-
 private:
   const lldb_private::RegisterInfo *m_register_info_p;
   uint32_t m_register_info_count;
   uint32_t m_user_register_count;
-  std::vector d_register_infos;
+  lldb_private::RegisterInfo m_orig_rax_info;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
@@ -152,32 +152,24 @@
 
 RegisterContextLinux_x86_64::RegisterContextLinux_x86_64(
 const ArchSpec &target_arch)
-: lldb_private::RegisterInfoInterface(target_arch),
+: lldb_private::RegisterContextLinux_x86(
+  target_arch,
+  {"orig_rax",
+   nullptr,
+   sizeof(((GPR *)nullptr)->orig_rax),
+   (LLVM_EXTENSION offsetof(GPR, orig_rax)),
+   eEncodingUint,
+   eFormatHex,
+   {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
+   nullptr,
+   nullptr}),
   m_register_info_p(GetRegisterInfoPtr(target_arch)),
   m_register_info_count(GetRegisterInfoCount(target_arch)),
-  m_user_register_count(GetUserRegisterInfoCount(target_arch)) {
-  RegisterInfo orig_ax = {
-  "orig_rax",
-  nullptr,
-  sizeof(((GPR *)nullptr)->orig_rax),
-  (LLVM_EXTENSION offsetof(GPR, orig_rax)),
-  eEncodingUint,
-  eFormatHex,
-  {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
-   LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM},
-  nullptr,
-  nullptr,
-  };
-  d_register_infos.push_back(orig_ax);
-}
+  m_user_register_count(GetUserRegisterInfoCount(target_arch)) {}
 
 size_t RegisterContextLinux_x86_64::GetGPRSizeStatic() { return sizeof(GPR); }
 
-const std::vector *
-RegisterContextLinux_x86_64::GetDynamicRegisterInfoP() const {
-  return &d_register_infos;
-}
-
 const RegisterInfo *Registe

[Lldb-commits] [PATCH] D141605: [lldb] Detach the child process when stepping over a fork

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf9e1fa17843: [lldb] Detach the child process when stepping 
over a fork (authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141605/new/

https://reviews.llvm.org/D141605

Files:
  lldb/source/Target/ThreadPlan.cpp
  lldb/test/API/functionalities/fork/resumes-child/Makefile
  lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
  lldb/test/API/functionalities/fork/resumes-child/main.cpp


Index: lldb/test/API/functionalities/fork/resumes-child/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/fork/resumes-child/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main() {
+  pid_t fork_result = fork(); // break here
+  assert(fork_result >= 0);
+  if (fork_result == 0) {
+// child
+_exit(47);
+  }
+  // parent
+  // Use polling to avoid blocking if the child is not actually resumed.
+  auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10);
+  std::chrono::milliseconds poll_interval{10};
+  while (std::chrono::steady_clock::now() < deadline) {
+int status;
+pid_t waitpid_result = waitpid(fork_result, &status, WNOHANG);
+if (waitpid_result == fork_result)
+  return 0;
+assert(waitpid_result == 0);
+std::this_thread::sleep_for(poll_interval);
+poll_interval *= 2;
+  }
+  abort();
+}
Index: lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
===
--- /dev/null
+++ lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
@@ -0,0 +1,22 @@
+"""
+Make sure that the fork child keeps running.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestForkResumesChild(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_step_over_fork(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+self.runCmd("next")
+self.expect("continue", substrs = ["exited with status = 0"])
Index: lldb/test/API/functionalities/fork/resumes-child/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/fork/resumes-child/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -171,6 +171,9 @@
   case eStopReasonExec:
   case eStopReasonThreadExiting:
   case eStopReasonInstrumentation:
+  case eStopReasonFork:
+  case eStopReasonVFork:
+  case eStopReasonVForkDone:
 return true;
   default:
 return false;


Index: lldb/test/API/functionalities/fork/resumes-child/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/fork/resumes-child/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main() {
+  pid_t fork_result = fork(); // break here
+  assert(fork_result >= 0);
+  if (fork_result == 0) {
+// child
+_exit(47);
+  }
+  // parent
+  // Use polling to avoid blocking if the child is not actually resumed.
+  auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10);
+  std::chrono::milliseconds poll_interval{10};
+  while (std::chrono::steady_clock::now() < deadline) {
+int status;
+pid_t waitpid_result = waitpid(fork_result, &status, WNOHANG);
+if (waitpid_result == fork_result)
+  return 0;
+assert(waitpid_result == 0);
+std::this_thread::sleep_for(poll_interval);
+poll_interval *= 2;
+  }
+  abort();
+}
Index: lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
===
--- /dev/null
+++ lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
@@ -0,0 +1,22 @@
+"""
+Make sure that the fork child keeps running.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestForkResumesChild(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_step_over_fork(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
+self.runCmd("next")
+self.expect("continue", substrs = ["exited with status = 0"])
Index: lldb/test/API/functionalities/fork/resumes-child/Makefile

[Lldb-commits] [lldb] af9e1fa - [lldb] Detach the child process when stepping over a fork

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-04-05T13:25:43+02:00
New Revision: af9e1fa178433653eb3d36c42cad016449873cfc

URL: 
https://github.com/llvm/llvm-project/commit/af9e1fa178433653eb3d36c42cad016449873cfc
DIFF: 
https://github.com/llvm/llvm-project/commit/af9e1fa178433653eb3d36c42cad016449873cfc.diff

LOG: [lldb] Detach the child process when stepping over a fork

Step over thread plans were claiming to explain the fork stop reasons,
which prevented the default fork logic (detaching from the child
process) from kicking in. This patch changes that.

Differential Revision: https://reviews.llvm.org/D141605

Added: 
lldb/test/API/functionalities/fork/resumes-child/Makefile
lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
lldb/test/API/functionalities/fork/resumes-child/main.cpp

Modified: 
lldb/source/Target/ThreadPlan.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlan.cpp 
b/lldb/source/Target/ThreadPlan.cpp
index 9913ecb591fa7..7927fc3140145 100644
--- a/lldb/source/Target/ThreadPlan.cpp
+++ b/lldb/source/Target/ThreadPlan.cpp
@@ -171,6 +171,9 @@ bool 
ThreadPlan::IsUsuallyUnexplainedStopReason(lldb::StopReason reason) {
   case eStopReasonExec:
   case eStopReasonThreadExiting:
   case eStopReasonInstrumentation:
+  case eStopReasonFork:
+  case eStopReasonVFork:
+  case eStopReasonVForkDone:
 return true;
   default:
 return false;

diff  --git a/lldb/test/API/functionalities/fork/resumes-child/Makefile 
b/lldb/test/API/functionalities/fork/resumes-child/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/functionalities/fork/resumes-child/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py 
b/lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
new file mode 100644
index 0..70df4f1c51a7c
--- /dev/null
+++ b/lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py
@@ -0,0 +1,22 @@
+"""
+Make sure that the fork child keeps running.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestForkResumesChild(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_step_over_fork(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+self.runCmd("next")
+self.expect("continue", substrs = ["exited with status = 0"])

diff  --git a/lldb/test/API/functionalities/fork/resumes-child/main.cpp 
b/lldb/test/API/functionalities/fork/resumes-child/main.cpp
new file mode 100644
index 0..2d37f323bdf6a
--- /dev/null
+++ b/lldb/test/API/functionalities/fork/resumes-child/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main() {
+  pid_t fork_result = fork(); // break here
+  assert(fork_result >= 0);
+  if (fork_result == 0) {
+// child
+_exit(47);
+  }
+  // parent
+  // Use polling to avoid blocking if the child is not actually resumed.
+  auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10);
+  std::chrono::milliseconds poll_interval{10};
+  while (std::chrono::steady_clock::now() < deadline) {
+int status;
+pid_t waitpid_result = waitpid(fork_result, &status, WNOHANG);
+if (waitpid_result == fork_result)
+  return 0;
+assert(waitpid_result == 0);
+std::this_thread::sleep_for(poll_interval);
+poll_interval *= 2;
+  }
+  abort();
+}



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 933d3ee - [lldb] Drop RegisterInfoInterface::GetDynamicRegisterInfo

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-04-05T13:25:43+02:00
New Revision: 933d3ee60007f5798319cad05b981cb265578ba0

URL: 
https://github.com/llvm/llvm-project/commit/933d3ee60007f5798319cad05b981cb265578ba0
DIFF: 
https://github.com/llvm/llvm-project/commit/933d3ee60007f5798319cad05b981cb265578ba0.diff

LOG: [lldb] Drop RegisterInfoInterface::GetDynamicRegisterInfo

"Dynamic register info" is a very overloaded term, and this particular
instance of it was only used for passing the information about the
"orig_[re]ax" pseudo-register on x86 through some generic code. Since
both sides of the code are x86-specific, I have replaced this with a
more direct route.

Differential Revision: https://reviews.llvm.org/D147045

Added: 
lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h

Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h
lldb/source/Plugins/Process/Utility/RegisterContextLinux_s390x.cpp
lldb/source/Plugins/Process/Utility/RegisterContextLinux_s390x.h
lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h
lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
index 4f1d2fab50f2b..e81ef3301f1f1 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -263,17 +263,17 @@ 
NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) {
 
 // NativeRegisterContextLinux_x86_64 members.
 
-static RegisterInfoInterface *
+static std::unique_ptr
 CreateRegisterInfoInterface(const ArchSpec &target_arch) {
   if (HostInfo::GetArchitecture().GetAddressByteSize() == 4) {
 // 32-bit hosts run with a RegisterContextLinux_i386 context.
-return new RegisterContextLinux_i386(target_arch);
+return std::make_unique(target_arch);
   } else {
 assert((HostInfo::GetArchitecture().GetAddressByteSize() == 8) &&
"Register setting path assumes this is a 64-bit host");
 // X86_64 hosts know how to work with 64-bit and 32-bit EXEs using the
 // x86_64 register context.
-return new RegisterContextLinux_x86_64(target_arch);
+return std::make_unique(target_arch);
   }
 }
 
@@ -297,7 +297,7 @@ static std::size_t GetXSTATESize() {
 NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)),
+  native_thread, CreateRegisterInfoInterface(target_arch).release()),
   NativeRegisterContextLinux(native_thread),
   NativeRegisterContextDBReg_x86(native_thread),
   m_xstate_type(XStateType::Invalid), m_ymm_set(), m_mpx_set(),
@@ -757,13 +757,8 @@ Status 
NativeRegisterContextLinux_x86_64::ReadAllRegisterValues(
* **/
 
   RegisterValue value((uint64_t)-1);
-  const RegisterInfo *reg_info =
-  GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_eax");
-  if (reg_info == nullptr)
-reg_info = GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_rax");
-
-  if (reg_info != nullptr)
-return DoWriteRegisterValue(reg_info->byte_offset, reg_info->name, value);
+  const RegisterInfo &info = GetRegisterInfo().GetOrigAxInfo();
+  return DoWriteRegisterValue(info.byte_offset, info.name, value);
 
   return error;
 }

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
index 43d939da84a39..40d086e0811bb 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
@@ -13,6 +13,7 @@
 
 #include "Plugins/Process/Linux/NativeRegisterContextLinux.h"
 #include "Plugins/Process/Utility/NativeRegisterContextDBReg_x86.h"
+#include "Plugins/Process/Utility/RegisterContextLinux_x86.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/Utility/lldb-x86-register-enums.h"
 #include 
@@ -130,6 +131,11 @@ class NativeRegisterContextLinux_x86_64
   bool IsMPX(uint32_t reg_index) const;
 
   void UpdateXSTATEforWrite(uint32_t reg_index);
+
+  RegisterContextLinux_x86 &GetRegisterInfo() const {
+return static_cast(
+*m_register_info_interface_up);
+  }
 };
 
 } // namespace process_linux

diff  --git a/lldb/source/Plugins/Process/Utility