[symbolizer] Address starting with a plus sign is valid.

[da-viper]

this is also the same behaviour in gnu addr2line.

depends on #135856

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Ebuka Ezike (da-viper)

Changes

this is also the same behaviour in gnu addr2line.

depends on #135856

---

Full diff: https://github.com/llvm/llvm-project/pull/135857.diff

2 Files Affected:

• (modified) llvm/test/tools/llvm-symbolizer/symbol-search.test (+3-2)
• (modified) llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp (+6-3)

diff --git a/llvm/test/tools/llvm-symbolizer/symbol-search.test b/llvm/test/tools/llvm-symbolizer/symbol-search.test
index 6729c4b01bfef..fe9a61bd8ef6c 100644
--- a/llvm/test/tools/llvm-symbolizer/symbol-search.test
+++ b/llvm/test/tools/llvm-symbolizer/symbol-search.test
@@ -65,8 +65,9 @@ RUN: llvm-symbolizer --obj=%p/Inputs/symbols.so func_01+0A | FileCheck --check-p
 RUN: llvm-addr2line --obj=%p/Inputs/symbols.so func_01+0A | FileCheck --check-prefix=NONEXISTENT %s
 
 # If '+' is not preceded by a symbol, it is part of a symbol name, not an offset separator.
-RUN: llvm-symbolizer --obj=%p/Inputs/symbols.so +0x1138 | FileCheck --check-prefix=NONEXISTENT %s
-RUN: llvm-addr2line --obj=%p/Inputs/symbols.so +0x1138 | FileCheck --check-prefix=NONEXISTENT %s
+# address starting with a `+` sign is a valid address
+RUN: llvm-symbolizer --obj=%p/Inputs/symbols.so +0x1138 | FileCheck --check-prefix=CODE-CMD %s
+RUN: llvm-addr2line --obj=%p/Inputs/symbols.so +0x1138 | FileCheck --check-prefix=CODE-CMD %s
 
 # Show that C++ mangled names may be specified.
 RUN: llvm-addr2line --obj=%p/Inputs/symbols.so _ZL14static_func_01i | FileCheck --check-prefix=MULTI-CXX %s
diff --git a/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp b/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
index 3ba7f59d5b847..c8abddbbab5dd 100644
--- a/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
+++ b/llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
@@ -238,9 +238,12 @@ static Error parseCommand(StringRef BinaryName, bool IsAddr2Line,
   bool StartsWithDigit = std::isdigit(AddrSpec.front());
 
   // GNU addr2line assumes the address is hexadecimal and allows a redundant
-  // "0x" or "0X" prefix; do the same for compatibility.
-  if (IsAddr2Line)
-    AddrSpec.consume_front("0x") || AddrSpec.consume_front("0X");
+  // "0x" or "0X" prefix or with an optional `+` sign; do the same for
+  // compatibility.
+  if (IsAddr2Line) {
+    AddrSpec.consume_front_insensitive("0x") ||
+        AddrSpec.consume_front_insensitive("+0x");
+  }
 
   // If address specification is a number, treat it as a module offset.
   if (!AddrSpec.getAsInteger(IsAddr2Line ? 16 : 0, Offset)) {

[jh7370]

@da-viper, why was this PR closed?

[da-viper]

@jh7370 I could not tell if the existing behaviour was intended since I did not see any response,
I assumed it was not that is why I closed it.

If it is not the case I can reopen it.

[jh7370]

Normally people "ping" a PR if it hasn't had any review after a week. You should also add some reviewers, by (if you have commit access) modifying the reviewers list, or (if you don't) pinging them here. I'd typically have reviewed this myself by now, but I'm on a reviewing pause, due to non-LLVM workloads being high at the moment.

I suggest you reopen and update (if needed). We aim for compatibility with the GNU tools wherever possible, so I'd guess this PR has merit (but I can't be sure without digging deeper, which I won't have time for for a couple more weeks probably).

[spavloff]

Could you please provide more context? What problem does this change solve? I checked the trunk version of addr2line, it seems it does not support such syntax.

[da-viper]

@spavloff

Could you please provide more context? What problem does this change solve?

When you pass and adress prefixed with + e.g.

$ llvm-addr2line --exe=./llvm-project/llvm/test/tools/llvm-symbolizer/Inputs/symbols.so +0x1138

??:0                                                                                                                                                                     [0.08s]
$ addr2line --exe=./llvm-project/llvm/test/tools/llvm-symbolizer/Inputs/symbols.so +0x1138      
/tmp/dbginfo/symbols.part1.cpp:12   

$ addr2line --version                                                                                                  
GNU addr2line version 2.44-3.fc42
Copyright (C) 2025 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

[spavloff]

This change does not add any new capabilities; it is purely syntactic sugar. Syntactic sugar is OK when it provides additional convenience, such as shorter commands or improved readability. Typically, there is no intention to merely expand the set of allowed constructs without clear benefits.

Addresses, by their nature, are unsigned. Using a plus sign in address specifications does not seem useful. Supporting unnecessary syntax may even introduce errors. For example, if a script invokes llvm-symbolizer with arguments like $BASE+1234, and the script doesn't define $BASE due to an error, the address would resolve to +1234. In the original implementation, this would raise an error, but with this change, it could output garbage instead.

Before adopting this patch, we need understand how this change could be useful.

[jh7370]

This change does not add any new capabilities; it is purely syntactic sugar. Syntactic sugar is OK when it provides additional convenience, such as shorter commands or improved readability. Typically, there is no intention to merely expand the set of allowed constructs without clear benefits.

Addresses, by their nature, are unsigned. Using a plus sign in address specifications does not seem useful. Supporting unnecessary syntax may even introduce errors. For example, if a script invokes llvm-symbolizer with arguments like $BASE+1234, and the script doesn't define $BASE due to an error, the address would resolve to +1234. In the original implementation, this would raise an error, but with this change, it could output garbage instead.

Before adopting this patch, we need understand how this change could be useful.

@da-viper has already highlighted that GNU addr2line supports this syntax. As we broadly aim for binary tools to be compatible with GNU tools where possible, we should accept this syntax too. The usefulness is that it then allows LLVM tools to be used as drop-in replacements for GNU tools in more cases. I will start properly reviewing this in a couple of weeks, if I don't get to it sooner.

Addresses, by their nature, are unsigned.

Aside: while I suspect this is true for all architectures LLVM supports, it's not an inherent requirement. Imagine an architecture where the address values are mapped one-to-one with physical elements in a memory array. There's nothing stopping the maker of that architecture labelling the "middle" one as value 0, therefore necessitating negative addresses.

[spavloff]

Sorry, I looked at wrong place.

LGTM.
Thanks!

[jh7370]

Some post-commit comments. Please create a follow-up PR to fix them.

In addition to the inline comments:

• Please update llvm/docs/CommandGuide/llvm-addr2line to mention the different behaviour.
• Please add a release note to highlight the change in behaviour, to the "Changes in LLVM Tools" section of llvm/docs/ReleaseNotes.md.

[jh7370]

Nit: comments should use a capital letter and trailing full stop.

[jh7370]

Please delete "with" from this comment.