On 10/27/2017 09:17 PM, Simon Marchi wrote:
On 2017-10-23 07:19 PM, Mike Gulick wrote:
...
Thanks a lot. Could you also write the corresponding ChangeLog
entries? See
here for details: http://sourceware.org/gdb/wiki/ContributionChecklist
Note that we usually put that as part of the commit log, and only move
it
to the actual ChangeLog files before pushing to git. The changes to
the
source files will go in gdb/ChangeLog and the changes to the testsuite
in
gdb/testsuite/ChangeLog.
You can also put the PR number as part of the ChangeLog (see wiki and
previous CL entries for examples).
Done.
I don't think you have a copyright assignment for GDB yet? Since this
is
more than a few lines, you will need one if you want to contribute
(I'll
contact you off-list for that).
I'm working on this. Hopefully it will be sooner rather than later.
I noted a a few comments below (mostly minor/formatting stuff, overall
I
think this is a good quality submission).
...
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index cc02740..b33de07 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -702,9 +702,14 @@ gdb_bfd_map_section (asection *sectp,
bfd_size_type *size)
data = NULL;
if (!bfd_get_full_section_contents (abfd, sectp, &data))
- error (_("Can't read data for section '%s' in file '%s'"),
- bfd_get_section_name (abfd, sectp),
- bfd_get_filename (abfd));
+ {
+ warning (_("Can't read data for section '%s' in file '%s'"),
+ bfd_get_section_name (abfd, sectp),
+ bfd_get_filename (abfd));
+ /* Section is invalid -- set size to 0 and return NULL */
Finish the comment with a dot and two spaces.
I rewrote the comment to be a full sentence
...
+int bar (int y) {
+ return y + 1; /* break here */
+}
Same here.
...
+#ifndef VANISH_LIB
+#define VANISH_LIB "./solib-vanish-lib1.so"
+#endif
I think we can remove this, it could only obfuscate a problem
if we remove the -DVANISH_LIB in the .exp file.
Done.
+
+int main()
Put "int" on its own line.
+{
+ void *handle;
+ int (*foo)(int);
+ char *error;
+
+ /* Open library */
+ handle = dlopen(VANISH_LIB, RTLD_NOW);
+ if (!handle) {
+ fprintf(stderr, "%s\n", dlerror());
Please make this file follow GNU style as well (space before
parenthesis,
curly braces position, etc).
+ exit(EXIT_FAILURE);
+ }
+
+ /* Clear any existing error */
+ dlerror();
+
+ /* Remove library */
+ if (remove(VANISH_LIB) == -1) {
Can we rename the lib instead of deleting it? It's always preferable
to keep
test artifacts in case we need to inspect them.
I updated the test to rename the library instead of removing it. It
also now renames it
back before exiting. This *should* make the test idempotent, unless
it fails before it
is able to rename the .so back to its original name.
...
+
+# Library 1
+set lib1name "solib-vanish-lib1"
+set srcfile_lib1 ${srcdir}/${subdir}/${lib1name}.c
+set binfile_lib1 [standard_output_file ${lib1name}.so]
+# I can't make this work for some reason:
+#set lib1_flags [list debug shlib=${binfile_lib2}]
+set lib1_flags [list debug ldflags=-l:${lib2name}.so
ldflags=-Wl,-rpath=[standard_output_file ""]
libdir=[standard_output_file ""]]
With
https://sourceware.org/ml/gdb-patches/2017-10/msg00856.html
the commented out line should work.
This did indeed work. Thank you!
...
+
+gdb_test "continue" \
+ "Breakpoint .*at .*/${lib2name}.c:${lib2_lineno}.*break here.*"
\
+ "breakpoint prints location"
I think you could use the "gdb_continue_to_breakpoint" proc.
This works as well, so the new patch uses this instead.
Here's an updated patch which hopefully addresses all of these items.
...