[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

[Djordje Todorovic via Phabricator via lldb-commits]

djtodoro added a comment.

In D79491#2024771 <https://reviews.llvm.org/D79491#2024771>, @labath wrote:

> In D79491#2024647 <https://reviews.llvm.org/D79491#2024647>, @djtodoro wrote:
>
> > Thanks a lot for this!
> >
> > > Nevertheless, I am still interested in making assembly-based tests for this (and similar features) because it enables testing scenarios that we could not get (reliably or at all) a compiler to produce.
> >
> > I also think this would be more stable if we can make assembler-based tests (but we'll need to address all archs from {x86_64, arm, aarch64}).
> >  I am just wondering, what are the obstacles for writing the assembler-based tests? Is it LLDB testing infrastructure or writing tests itself?
>
>
> A bit of both, maybe. Writing a test which works on a single target (os+arch) was relatively easy (for me, because I've done a lot of this stuff lately, maybe not so easy for others), but the difficulties started when I wanted to make that test run on other oses (which have different asm dialects). I was ok with leaving the test x86-specific, since most developers have an x86 machine and there is nothing arch-specific about this functionality. However, I did not want to restrict the test to any single OS, and since this test requires a running program, the asm needed to match what the host expects.
>
> I did manage to ifdef around the asm platform quirks, but I still haven't managed to get the darwin linker to recognize my hand-written dwarf. I am sure this can be fixed (I think it's  because I reduced the input too much), and I do want to try it out, but I am not sure the result will be something we can recommend as a general practice.


I see.. Enabling it only for specific OSes  could be a temp solution, but we can discuss about that.

> A different way to address this would be to remove the requirement for a running process. The test doesn't really require that, it just needs a relatively complex static snapshot of the process. A core file is just that, but we currently don't have any good way of creating such core files. If we had that, we could run this test over a core file. That would still be platform specific, but then it wouldn't matter (so much) because it wouldn't be tied to the host platform.

Good idea! A corefile could be solution, and I believe LLDB parses corefiles from other architectures very well (as multiarch GDB does).



================
Comment at: lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py:5
+
+supported_archs = ["x86_64", "arm", "aarch64"]
+
----------------
labath wrote:
> djtodoro wrote:
> > I haven't refreshed the page before submitting my previous comment.
> > 
> > WDYT about adding a decorators-function (e.g. `skipUnlessEntryValuesSupportedArch`) so we can use it in other tests as well?
> I think that's a good idea, and that the check can be folded into the existing `skipUnlessHasCallSiteInfo` decorator.
Please note that entry values and call site info are not the same thing. Entry values implementation ended up describing target-specific instructions (in terms of DWARF operations) that loads values into registers that transfers the parameters, and that is used for the entry values printing. Eventually, we will have all targets supporting the entry values, but the call site Info is something different. The call site info is controlled by the `DIFlagAllCallsDescribed` flag, and it is generated by the language front-end, according to DWARF standard. Therefore, it makes sense to me to add new independent function (`skipUnlessEntryValuesSupportedArch`), but the whole feature depends on both `skipUnlessEntryValuesSupportedArch` and `skipUnlessHasCallSiteInfo`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491