[Lldb-commits] [PATCH] D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors

Thu May 9 07:42:22 PDT 2019

sgraenitz marked 2 inline comments as done.
sgraenitz added a comment.

@stella.stamenova Can you have a look at the lit test please? It works on macOS and Linux, but I didn't test Windows. Should I add something like `# REQUIRES: nowindows` or is it fine like this?

================
Comment at: lldb/lit/Breakpoint/jitbp_elf.test:1
+# REQUIRES: target-x86_64, system-linux, native
+
----------------
labath wrote:
> sgraenitz wrote:
> > The test only works with ELF on Linux. Is the `REQUIRES` sufficient?
> Yes, but what is the reason for that? It looks like the test is generic enough that it should run on any system where lli is able to jit code. In particular I'd expect this to also work on macOS if you set `plugin-jit-loader.gdb.enable` to `on`.
Indeed, since https://reviews.llvm.org/D57689 it does work on macOS with ELF.

================
Comment at: lldb/lit/Breakpoint/jitbp_elf.test:3
+
+# RUN: %clang -g -S -emit-llvm -o %t.ll %p/Inputs/jitbp.cpp
+# RUN: %lldb -b -o 'b jitbp' -o 'run -jit-kind=mcjit %t.ll' lli | FileCheck %s
----------------
labath wrote:
> sgraenitz wrote:
> > With these args, clang shouldn't optimize away `jitbp()`
> Is that a question? If it is I'm pretty sure the answer is "it shouldn't", because a lot of our tests depend on -O0 not doing anything funny.
Yes, I didn't know whether `-g` is enough or I better pass something like `-O0` explicitly?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61611