[v2,gdb/python] Bploc should try to return full path

Message ID 20250113212723.25234-1-simon.farre.cx@gmail.com
State New
Headers
Series [v2,gdb/python] Bploc should try to return full path |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Simon Farre Jan. 13, 2025, 9:27 p.m. UTC
  v2.

Use symtab_to_fullname instead of calling fullname (),
because this will attempt to resolve the path if it hasn't already been
resolved.

v1.

Compilers often emit relative paths in the line number program,
relative to the build directory for that compilation unit (if it's
DWARF>=4 I think).

Therefore use symtab->fullname() when not null as this seemingly
has attempted path normalization for the symtab and only
fall back on symtab->filename which will never be null if that fails.

This has a much better UX. Applications may choose to expose
this name as a clickable link to some file, at which point
a non-normalized and non-absolute path would lead nowhere.

When I wrote this feature the first time, I don't think this
relative-to-cu-scheme was as prevalent in the output of gcc/clang
for DWARF.
---
 gdb/python/py-breakpoint.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

Comments

Tom Tromey Jan. 14, 2025, 4:59 p.m. UTC | #1
>>>>> "Simon" == Simon Farre <simon.farre.cx@gmail.com> writes:

Simon> -      /* symtab->filename is never NULL. */
Simon> -      gdbpy_ref<> filename
Simon> -	= host_string_to_python_string (self->bp_loc->symtab->filename);
Simon> +      const char *full = symtab_to_fullname (self->bp_loc->symtab);

It's hard to tell if symtab_to_fullname can throw an exception.
If it can, though, then this code has to be wrapped in a try/catch.

It's safe to just add the try/catch, just in case.

Simon> +      gdbpy_ref<> filename = full != nullptr
Simon> +        ? host_string_to_python_string (full)
Simon> +        : host_string_to_python_string (self->bp_loc->symtab->filename);

This needs parens around the RHS of the assignment to conform to gdb
formatting rules.

thanks,
Tom
  

Patch

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 75f50e1f423..f22df079f90 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -17,6 +17,7 @@ 
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "source.h"
 #include "value.h"
 #include "python-internal.h"
 #include "python.h"
@@ -1643,9 +1644,10 @@  bplocpy_get_source_location (PyObject *py_self, void *closure)
       gdbpy_ref<> tup (PyTuple_New (2));
       if (tup == nullptr)
 	return nullptr;
-      /* symtab->filename is never NULL. */
-      gdbpy_ref<> filename
-	= host_string_to_python_string (self->bp_loc->symtab->filename);
+      const char *full = symtab_to_fullname (self->bp_loc->symtab);
+      gdbpy_ref<> filename = full != nullptr
+        ? host_string_to_python_string (full)
+        : host_string_to_python_string (self->bp_loc->symtab->filename);
       if (filename == nullptr)
 	return nullptr;
       auto line = gdb_py_object_from_ulongest (self->bp_loc->line_number);