[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

DavidSpickett created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. DavidSpickett requested review of this revision. Herald added a subscriber: JDevlieghere. init_llgs_test no longer takes an argument but these two were not updated. Also fix some mistakes i

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

DavidSpickett added a reviewer: JDevlieghere. DavidSpickett added a comment. I presume these tests aren't commonly run or have been masked by the expected failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91612/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

DavidSpickett added a reviewer: labath. DavidSpickett added a comment. For reference the argument was removed in: https://reviews.llvm.org/D90313 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91612/new/ https://reviews.llvm.org/D91612

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. DavidSpickett requested review of this revision. Herald added a subscriber: JDevlieghere. Previously if you did: $ lldb-server platform --server <...> --min-gdbserver-port 12346 --max-gdbser

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett added a comment. I have found existing tests that launch an lldb-server with specific arguments, using an existing lldb-server platform to do it. The ideal test would be: - run lldb-server with min port some known to be available port, max is that +1 - connect the client - send qL

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

This revision was automatically updated to reflect the committed changes. Closed by commit rG50f12ade2de1: [lldb] Fix a couple of remote llgs tests (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91612/new/ https://reviews

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett added a comment. I hadn't considered using a unit test instead, I'll give that a go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91634/new/ https://reviews.llvm.org/D91634 ___ lldb-commit

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

DavidSpickett updated this revision to Diff 306091. DavidSpickett added a comment. Updated the way the test works. - If you build without MTE in the toolchain the binary just returns the magic failure number. - If you do have an MTE toolchain but your target doesn't have MTE then it will also f

[Lldb-commits] [PATCH] D87442: [lldb][AArch64/Linux] Show memory tagged memory regions

DavidSpickett updated this revision to Diff 306406. DavidSpickett added a comment. - Use assertIn - Cleanup Status/Error handling - Refactor test so it runs once and we skip based on whether it returns a value or hits a breakpoint. - Seperated exit codes for non MTE toolchain and non MTE target,

[Lldb-commits] [PATCH] D87442: [lldb][AArch64/Linux] Show memory tagged memory regions

DavidSpickett added a comment. I'm going to stick with the test format as it is. No doubt I'll need to clean it up based on what future MTE tests look like. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87442/new/ https://reviews.llvm.org/D87442

[Lldb-commits] [PATCH] D87442: [lldb][AArch64/Linux] Show memory tagged memory regions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG32541685b2a9: [lldb][AArch64/Linux] Show memory tagged memory regions (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

DavidSpickett added a comment. FYI my change to add the memory tagging flag (https://reviews.llvm.org/D87442) has landed so you will have some conflicts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88387/new/ https://reviews.llvm.org/D88387 ___

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett updated this revision to Diff 307356. DavidSpickett added a comment. Herald added a subscriber: mgorny. - Move PortMap into a class we can unittest - Use llvm::Expected for GetNextAvailablePort Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[Lldb-commits] [PATCH] D92035: [lldb] Use llvm::Optional for port in LaunchGDBServer

DavidSpickett created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. DavidSpickett requested review of this revision. Herald added a subscriber: JDevlieghere. Previously we used UINT16_MAX to mean no port/no specifc port. This leads to confusion because 65535

[Lldb-commits] [PATCH] D92035: [lldb] Use llvm::Optional for port in LaunchGDBServer

DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp:197 #endif - uint16_t *port_ptr = &port; + uint16_t *port_ptr = port.getPointer(); if (m_socket_protocol == Socket::ProtocolTcp) { -

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h:30 public: - using PortMap = std::map; using PacketHandler = Only the one in GDBRemoteCommunicationServerPlatform is/was used. Repositor

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett updated this revision to Diff 307822. DavidSpickett added a comment. - Use auto for map find - Document PortMap functions - Remove unnecessary casts - Move tests to Process/gdb-remote - Cleanup Expected asserts The behaviour of AssociatePortWithProcess is a bit non obvious (that it

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett marked 6 inline comments as done. DavidSpickett added inline comments. Comment at: lldb/tools/lldb-server/lldb-platform.cpp:234 if (ch == 'P') -gdbserver_portmap[portnum] = LLDB_INVALID_PROCESS_ID; +gdbserver_portmap.AllowPort(static_cast(port

[Lldb-commits] [PATCH] D92035: [lldb] Use llvm::Optional for port in LaunchGDBServer

DavidSpickett updated this revision to Diff 307824. DavidSpickett added a comment. - Reformat GDBRemoteCommunicationServerPlatform.h to fix include order. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92035/new/ https://reviews.llvm.org/D92035 Fil

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

DavidSpickett updated this revision to Diff 308289. DavidSpickett added a comment. - Document default constructor behaviour. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91634/new/ https://reviews.llvm.org/D91634 Files: lldb/source/Plugins/Proc

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG98e87f76d0f4: [lldb] Error when there are no ports to launch a gdbserver on (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHAN

[Lldb-commits] [PATCH] D92035: [lldb] Use llvm::Optional for port in LaunchGDBServer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa7f8d96b16a3: [lldb] Use llvm::Optional for port in LaunchGDBServer (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINC

[Lldb-commits] [PATCH] D93225: [lldb] Add helper class for dealing with key:value; GDB responses

DavidSpickett created this revision. Herald added a subscriber: mgorny. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The current handling of qHostInfo is a bunch of if-else, one for each key we think we might find. This wor

[Lldb-commits] [PATCH] D93226: [lldb] Apply KeyValueExtractorGDBRemote to qProcessInfo

DavidSpickett created this revision. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The original logic is retained. Unlike qHostInfo, qProcessInfo does not check whether at least one key was parsed correctly. This allowed me

[Lldb-commits] [PATCH] D93225: [lldb] Add helper class for dealing with key:value; GDB responses

DavidSpickett added reviewers: labath, jasonmolenda. DavidSpickett added a comment. I realise it's a lot to review but if you have any initial comments on the usability that'd be great. My main point here was to make the if-else chain a bit "safer" and more easily extended. The parsing logic its

[Lldb-commits] [PATCH] D93225: [lldb] Add helper class for dealing with key:value; GDB responses

DavidSpickett planned changes to this revision. DavidSpickett added a comment. > Have you looked at the how llvm YAML and JSON libraries handle > (de)serialization? I was hoping that we could implement something similar to > that... Good point, I was only looking at half the issue. I'll see how

[Lldb-commits] [PATCH] D93495: CrashReason: Add MTE tag check faults to the list of crash reasons.

DavidSpickett added a comment. I assume that the signal displays without tag bits just like any other SEGV? I was thinking about testing but I don't see any tests for specific signals so it would only be needed if your change to add the tag bits to siginfo has gone in. (I have been following it

[Lldb-commits] [PATCH] D93495: CrashReason: Add MTE tag check faults to the list of crash reasons.

DavidSpickett added a comment. Great, thanks for the explanation. I don't think this needs a test since (for now) it acts just like existing signals. The change looks good to me but @labath should approve since I don't know all the signal handling details. Repository: rG LLVM Github Monore

[Lldb-commits] [PATCH] D94084: [lldb][ARM/AArch64] Update disasm flags to latest v8.7a ISA

DavidSpickett created this revision. Herald added subscribers: danielkiss, kristof.beyls. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Add optional memory tagging extension on AArch64. Use isAArch64() instead of listing th

[Lldb-commits] [PATCH] D94084: [lldb][ARM/AArch64] Update disasm flags to latest v8.7a ISA

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG801c7866e6d4: [lldb][ARM/AArch64] Update disasm flags to latest v8.7a ISA (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGE

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

DavidSpickett added a comment. Do you have python enabled in the build? (https://lldb.llvm.org/resources/build.html#preliminaries) The cmake option LLDB_ENABLE_PYTHON defaults to Auto which has tripped me up in the past. If you set it to YES then you'll get a build error if it fails to find a

[Lldb-commits] [PATCH] D94917: [lldb] Fix crash in "help memory read"

DavidSpickett created this revision. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When a command option does not have a short version (e.g. -f for --file), we use an arbitrary value in the short_option field to mark it as i

[Lldb-commits] [PATCH] D94917: [lldb] Fix crash in "help memory read"

DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. UBSAN might have caught this if we had a test that took this path but I need to confirm that. If so I could add some more as a smoke test of sorts, or just call help on every command if that's not going to take an age. See

[Lldb-commits] [PATCH] D94917: [lldb] Fix crash in "help memory read"

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9a7672ac4980: [lldb] Fix crash in "help memory read" (authored by DavidSpickett). Changed prior to commit: https://reviews.llvm.org/D94917?vs=3173

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

DavidSpickett created this revision. Herald added subscribers: danielkiss, kristof.beyls, krytarowski, mgorny. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This enables memory tag reading for lldb-server when on AArch64 Lin

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading for lldb

DavidSpickett created this revision. Herald added subscribers: danielkiss, kristof.beyls, mgorny. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This commit adds a new command "memory tag read". It looks much like "memory rea

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

DavidSpickett added reviewers: labath, omjavaid, emaste. DavidSpickett added a subscriber: emaste. DavidSpickett added a comment. This goes together with https://reviews.llvm.org/D95602, I've split them to make review more manageable. @emaste I saw you were doing something in BSD land around MTE

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading for lldb

DavidSpickett added reviewers: omjavaid, palebedev, emaste. DavidSpickett added a comment. Couple of things I wanted to highlight for review. I've put the tag handler on the architecture plugin, but it could also go on process directly like trace does. I figured tagging extensions are a per arch

[Lldb-commits] [PATCH] D96460: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:79 + m_mte_ctrl_reg = 0; + Should m_pac_mask also be initialised or is it already? Comment at: lldb/source/Plugins/Proc

<    11   12   13   14   15   16