[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

brucem created this revision. brucem requested review of this revision. Herald added a project: LLDB. This is another step towards implementing the equivalent of `platform process list` and related functionality. `uint32_t` is used for the argument count and index despite the underlying value bei

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

mib accepted this revision as: mib. mib added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103675/new/ https://reviews.llvm.org/D103675

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

teemperor added inline comments. Comment at: lldb/bindings/interface/SBProcessInfo.i:78 + +%feature("docstring", +"Return the specified argument given to the described process." Can you add this line here? ``` %feature("autodoc", "GetArgumentAtIndex(int

[Lldb-commits] [PATCH] D103652: [lldb][NFC] Refactor name to index maps in Symtab

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Only have some comments about the way `FindFunctionSymbols` is now implemented, but otherwise this LGTM. Comment at: lldb/include/lldb/Symbol/Symtab.h:177

[Lldb-commits] [lldb] 0a655c6 - [lldb][NFC] Remove a redundant call to weak_ptr::expired

Author: Raphael Isemann Date: 2021-06-04T12:06:53+02:00 New Revision: 0a655c62eca878cd5f366c08a4a5fee1b8723ce8 URL: https://github.com/llvm/llvm-project/commit/0a655c62eca878cd5f366c08a4a5fee1b8723ce8 DIFF: https://github.com/llvm/llvm-project/commit/0a655c62eca878cd5f366c08a4a5fee1b8723ce8.dif

[Lldb-commits] [PATCH] D103442: [lldb][NFC] Remove a redundant call to weak_ptr::expired

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0a655c62eca8: [lldb][NFC] Remove a redundant call to weak_ptr::expired (authored by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield updated this revision to Diff 347111. mbenfield added a comment. Move fixes into a separate change https://reviews.llvm.org/D102942. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 Files: clan

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added inline comments. Comment at: llvm/include/llvm/Support/TargetRegistry.h:26 #include "llvm/ADT/iterator_range.h" +#include "llvm/MC/MCObjectFileInfo.h" #include "llvm/Support/CodeGen.h" `include/llvm/Support/TargetRegistry.h now has cyclic dependen

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

flip1995 added inline comments. Comment at: llvm/include/llvm/Support/TargetRegistry.h:26 #include "llvm/ADT/iterator_range.h" +#include "llvm/MC/MCObjectFileInfo.h" #include "llvm/Support/CodeGen.h" MaskRay wrote: > `include/llvm/Support/TargetRegistry.h now h

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

flip1995 added inline comments. Comment at: llvm/include/llvm/Support/TargetRegistry.h:26 #include "llvm/ADT/iterator_range.h" +#include "llvm/MC/MCObjectFileInfo.h" #include "llvm/Support/CodeGen.h" flip1995 wrote: > MaskRay wrote: > > `include/llvm/Support/Ta

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added subscribers: compnerd, dblaikie. MaskRay added a comment. Because of `new MCObjectFileInfo`, we cannot use a forward declaration (incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in `TargetRegistry.h`. I thought about moving `TargetRegistry.{h,cpp}` from Suppor

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

dblaikie added a comment. In D101921#2786245 , @MaskRay wrote: > Because of `new MCObjectFileInfo`, we cannot use a forward declaration > (incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in > `TargetRegistry.h`. > > I thought about

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Removing from queue - i don't expect to review this. Looks like this has been reverted twice now, presumably llvm stage 2/linux kernel/chrome should be enough of a coverage to be s

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc2f819af73c5: [MC] Refactor MCObjectFileInfo initialization and allow targets to create… (authored by flip1995, committed by MaskRay). Repository:

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

sven added a comment. @luismarques I have tried the patch with llvm12, test case is provide by jade(https://gist.github.com/e2efac2f780ed820277dbaf608805f4e), but it didn't worked for me. After execute 'gdb-remote 1234', the terminal shows: Process 1 stopped * Thread #1, stop reason = sign

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

luismarques added a comment. In D62732#2789409 , @sven wrote: > I have tried the patch with llvm12, test case is provide by > jade(https://gist.github.com/e2efac2f780ed820277dbaf608805f4e), but it didn't > worked for me. > It seems that the unwind didn't

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

luismarques added a comment. In D62732#2790028 , @luismarques wrote: > That's surprising. I'll see if I can figure out what the issue might be. > Thanks. Confirmed. Something must have broken since the last patch revision. I'll see if I can figure out /

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

sven added a comment. In D62732#2790087 , @luismarques wrote: > In D62732#2790028 , @luismarques > wrote: > >> That's surprising. I'll see if I can figure out what the issue might be. >> Thanks. > > Confirmed. Som

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

jade added a comment. In D62732#2789409 , @sven wrote: > It seems that the unwind didn't succeed, can you figure out the problem? > my qemu vesion: 4.2.1 > Thanks. Unfortunately, from my last adventures through the lldb codebase in debugging that (see bu

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf49cae278b4: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable (authored by mbenfield, committed by george.burgess.iv). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. I think there is a false positive with this @george.burgess.iv: In [this)(https://searchfox.org/mozilla-central/source/mozglue/baseprofiler/core/platform-linux-android.cpp#222-227) we have the warning triggered: mozglue/baseprofiler/core/platform-linux-android.cpp:

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

xbolva00 added a comment. In D100581#2792854 , @Abpostelnicu wrote: > I think there is a false positive with this @george.burgess.iv: > In this >

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

ldionne added a comment. Hello! There are still some false positives, for example this one is breaking the libc++ CI: https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848 Would it be possible to either fix this quickly or revert the change until the iss

Re: [Lldb-commits] Prevent type canonization when dereferencing

I think the problem is specific to references, at least that's the case I ran into in lldb-eval -- https://github.com/google/lldb-eval/blob/master/docs/lldb-bugs.md#dereferencing-a-value-canonizes-the-type On Wed, 2 Jun 2021 at 17:26, Raphael “Teemperor” Isemann < teempe...@gmail.com> wrote: > I

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

dblaikie added a comment. In D100581#2793775 , @ldionne wrote: > Hello! There are still some false positives, for example this one is breaking > the libc++ CI: > https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a88

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2792854 , @Abpostelnicu wrote: > I think there is a false positive with this @george.burgess.iv: > In this >

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

ldionne added a comment. In D100581#2793926 , @dblaikie wrote: > In D100581#2793775 , @ldionne wrote: > >> Hello! There are still some false positives, for example this one is >> breaking the libc++ CI: >> https

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

sberg added a comment. Is it intentional that this warns about volatile variables as in void f(char * p) { volatile char c = 0; c ^= *p; } (I see that GCC warns about volatile too, at least when you replace the `^=` with `=`, so assume the answer is "yes", but would just like to

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

mbenfield added a comment. In D100581#2795966 , @sberg wrote: > Is it intentional that this warns about volatile variables as in Yes, volatile was intentional. Alright, I will add a test for this case. Repository: rG LLVM Github Monorepo CHANGES SIN

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

xbolva00 added a comment. In D100581#2798079 , @raj.khem wrote: > http://sprunge.us/FJzZXL is a file from harfbuzz and it warns > > a.cc:28670:32: error: variable 'supp_size' set but not used > [-Werror,-Wunused-but-set-variable] > unsigned int s

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

raj.khem added a comment. http://sprunge.us/FJzZXL is a file from harfbuzz and it warns a.cc:28670:32: error: variable 'supp_size' set but not used [-Werror,-Wunused-but-set-variable] unsigned int size0, size1, supp_size = 0; I do not have -Werror enabled but it still is reported as err

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. In D100581#2798079 , @raj.khem wrote: > http://sprunge.us/FJzZXL is a file from harfbuzz and it warns > > a.cc:28670:32: error: variable 'supp_size' set but not used > [-Werror,-Wunused-but-set-variable] > unsigned i

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

DavidSpickett created this revision. Herald added subscribers: pengfei, kristof.beyls. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There is a common pattern: result.AppendError(...); result.SetStatus(eReturnStatusFailed);

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. There are likely other tests that aren't enabled for x86/AArch64 or sets of registers that I don't have on my machines. So if this change is welcome then the plan would be to land this as is and monitor the bots for a week o

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

teemperor added a comment. I can run this for you on macOS and Linux x86 which I think should cover every test. FWIW, there is even more redundancy from what I remember as we IIRC return false within `bool DoExecute` everywhere. I can't recall why I didn't remove this yet (beside that it's a l

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

teemperor added a comment. Oh I see, you're not just concerned about just having every command in the test suite covered before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103701/new/ https://reviews.llvm.org/D103701 __

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

DavidSpickett added a comment. > I can run this for you on macOS and Linux x86 which I think should cover > every test. That would be great, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103701/new/ https://reviews.llvm.org/D103701 _

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

brucem updated this revision to Diff 349895. brucem added a comment. Rebase and address feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103675/new/ https://reviews.llvm.org/D103675 Files: lldb/bindings/interface/SBProcessInfo.i lldb/in

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

brucem marked an inline comment as done. brucem added inline comments. Comment at: lldb/bindings/interface/SBProcessInfo.i:78 + +%feature("docstring", +"Return the specified argument given to the described process." teemperor wrote: > Can you add this lin

[Lldb-commits] [PATCH] D103701: [lldb] Set return status to failed when adding a command error

jingham added a comment. This seems like a good change to me. And even if you wanted to do something cheesy like put an error into the result on spec, then later decide it was successful after all, you can always reset the result's status to the appropriate success status. This just makes the

[Lldb-commits] [PATCH] D103575: Allow signposts to take advantage of deferred string substitution

JDevlieghere added a comment. I think we should just remove the functionality form the timer class again. I only added it there because of the macro. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103575/new/ https://reviews.llvm.org/D103575

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. HandleCommands really does need to do this suppression, and this is a fine way to do it. I can't think of any other place where you would need to do this suppression after an admittedly non

[Lldb-commits] [lldb] 8d33437 - [LLDB/API] Expose args and env from SBProcessInfo.

Author: Bruce Mitchener Date: 2021-06-05T13:42:18+07:00 New Revision: 8d33437d030af27fff21dd3fd0e66893b0148217 URL: https://github.com/llvm/llvm-project/commit/8d33437d030af27fff21dd3fd0e66893b0148217 DIFF: https://github.com/llvm/llvm-project/commit/8d33437d030af27fff21dd3fd0e66893b0148217.dif

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. brucem marked an inline comment as done. Closed by commit rG8d33437d030a: [LLDB/API] Expose args and env from SBProcessInfo. (authored by brucem). Repository: rG LLV