labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.
Thanks for the updates and for writing the test. The code looks fine to me, the
"request changes" is for making sure the test follows best practices and is
portable.
================
Comment at: lldb/test/API/tools/lldb-vscode/breakpoint/Makefile:10-17
+# The following shared library will be used to test breakpoints under dynamic
loading
+libother.so: other-copy.c
+ $(CC) $(CFLAGS) -fpic -o other.o -c other-copy.c
+ $(CC) $(CFLAGS) -shared -o libother.so other.o
+
+a.out: main-copy.cpp libother.so
+ echo $(CXXFLAGS) > ./flags
----------------
This is not the right way to build binaries -- it will fail on macos for
instance.
If we were not dlopening, you should set CXX_SOURCES, and DYLIB_C_SOURCES and
let the normal rules do build for you, but for dlopen you need to do something
a tad more complicated and invoke a submake with the right arguments you can
look at `functionalities/load_unload/Makefile` for an example of how to do that.
I don't think we currently have a test which uses both shared libraries, and
relocated sources files, so it's possible that this may fail for some reason.
But if that's the case, then I want to understand what's the problem first (so
we can try to fix it).
================
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py:32-33
+
+ new_main_folder = os.path.join(os.path.dirname(source_folder),
'moved_main')
+ new_other_folder = os.path.join(os.path.dirname(source_folder),
'moved_other')
+
----------------
If I follow this correctly this will escape from the build folder for this test
(two `dirname`s). Can you stay within the confines of the build folder?
Among other things, then you won't need to `rmtree` these things as the build
folder is automatically cleaned before each test.
================
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py:229-230
breakpoint, like 'conditions' and 'hitCondition' settings.'''
- source_basename = 'main.cpp'
- source_path = os.path.join(os.getcwd(), source_basename)
+ source_basename = 'main-copy.cpp'
+ source_path = self.getBuildArtifact(source_basename)
loop_line = line_number('main.cpp', '// break loop')
----------------
maybe move these into the `setUp` function?
================
Comment at: lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp:19
int main(int argc, char const *argv[]) {
+ void *handle = dlopen("libother.so", RTLD_NOW);
+ int (*foo)(int) = (int (*)(int))dlsym(handle, "foo");
----------------
Looking at the other dlopen tests (e.g.,
`functionalities/load_unload/main.cpp`), it looks like you'll need to ifdef the
correct library name here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76968/new/
https://reviews.llvm.org/D76968
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
