[PATCH 0/7] Objective-C fixes

[PATCH 0/7] Objective-C fixes

[Tom Tromey]

This series fixes up the Objective-C tests, then fixes some other
Objective-C bugs.  All the functionality still doesn't really work,
but at least now gdb won't crash.

Tom

[PATCH 1/7] Reformat gdb.objc tests

[Tom Tromey]

This patch applies some minor formatting changes and cleanups to the
gdb.objc tests.
---
 gdb/testsuite/gdb.objc/basicclass.exp |  9 ++++-----
 gdb/testsuite/gdb.objc/nondebug.exp   |  9 ++++-----
 gdb/testsuite/gdb.objc/objcdecode.exp | 11 +++++------
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/gdb/testsuite/gdb.objc/basicclass.exp b/gdb/testsuite/gdb.objc/basicclass.exp
index 9e40c7138dc..d1fb64a42de 100644
--- a/gdb/testsuite/gdb.objc/basicclass.exp
+++ b/gdb/testsuite/gdb.objc/basicclass.exp
@@ -21,8 +21,9 @@ standard_testfile .m
 # Objective-C program compilation isn't standard. We need to figure out
 # which libraries to link in. Most of the time it uses pthread
 #
-if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug ]] != "" } {
-  return -1
+if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	 executable {debug}] != ""} {
+    return
 }
 
 #
@@ -147,7 +148,7 @@ gdb_test "break hiddenMethod" \
 #
 gdb_test continue \
     "Continuing\\..*Breakpoint \[0-9\]+, -.BasicClass\\(Private\\) hiddenMethod. \\(self=.*, _cmd=.*\\) at .*$srcfile:61.*" \
-     "continue until category method"
+    "continue until category method"
 
 #
 # Test calling Objective-C methods
@@ -170,5 +171,3 @@ gdb_test "print-object object" \
 gdb_test "po self" \
     "BasicClass gdb test object" \
     "use of the po (print-object) command"
-
-
diff --git a/gdb/testsuite/gdb.objc/nondebug.exp b/gdb/testsuite/gdb.objc/nondebug.exp
index 9ca86ff8189..321aa229045 100644
--- a/gdb/testsuite/gdb.objc/nondebug.exp
+++ b/gdb/testsuite/gdb.objc/nondebug.exp
@@ -23,8 +23,9 @@ standard_testfile .m
 # Objective-C program compilation isn't standard. We need to figure out
 # which libraries to link in. Most of the time it uses pthread
 #
-if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list ] ] != "" } {
-  return -1
+if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	 executable {}] != ""} {
+    return
 }
 
 proc do_objc_tests {} {
@@ -42,8 +43,7 @@ gdb_test "set breakpoint pending off" "" "set breakpoint pending"
 # Break on multiply defined non-debuggable symbol (PR objc/1236)
 #
 set name "break on non-debuggable method"
-gdb_test_multiple "break someMethod" $name \
-{
+gdb_test_multiple "break someMethod" $name {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\]\[ \]+-.NonDebug someMethod.*\\\[3\\\]\[ \]+-.NonDebug2 someMethod.*" {
 	send_gdb "2\n"
 	exp_continue
@@ -55,4 +55,3 @@ gdb_test_multiple "break someMethod" $name \
     -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+.*$gdb_prompt $" { pass $name }
     -re ".*$gdb_prompt $"   { kfail "gdb/1236" $name }
 }
-
diff --git a/gdb/testsuite/gdb.objc/objcdecode.exp b/gdb/testsuite/gdb.objc/objcdecode.exp
index 5414f0f96f3..7840a2ed5d3 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.exp
+++ b/gdb/testsuite/gdb.objc/objcdecode.exp
@@ -23,8 +23,9 @@ standard_testfile .m
 # Objective-C program compilation isn't standard. We need to figure out
 # which libraries to link in. Most of the time it uses pthread
 #
-if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug ]] != "" } {
-  return -1
+if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+	 {debug}] != ""} {
+    return
 }
 
 proc do_objc_tests {} {
@@ -40,8 +41,7 @@ gdb_test_no_output "set multiple-symbols ask"
 # Break on multiply defined method (PR objc/1236)
 #
 set name "break on multiply defined method"
-gdb_test_multiple "break multipleDef" $name \
-{
+gdb_test_multiple "break multipleDef" $name {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] .*${srcfile}:-.Decode multipleDef.\r\n\\\[3\\\] .*${srcfile}:multipleDef\r\n> $" {
 	send_gdb "3\n"
 	exp_continue
@@ -52,8 +52,7 @@ gdb_test_multiple "break multipleDef" $name \
 
 set name "continue after break on multiply defined symbol"
 gdb_run_cmd
-gdb_test_multiple "" $name \
-{
+gdb_test_multiple "" $name {
     -re "Breakpoint \[0-9\]+, multipleDef \\\(\\\) at .*\r\n$gdb_prompt $" {
 	pass $name
     }
-- 
2.49.0

[PATCH 2/7] Make the gdb.objc tests compile

[Tom Tromey]

The gdb.objc tests haven't compiled in years.  This patch fixes this,
based on a comment in bug 31671.

I don't know whether this approach works with the clang implementation
of Objective-C.  However, it does work with GCC, provided that
gnustep-base is installed.
---
 gdb/testsuite/gdb.objc/basicclass.m |  6 +++---
 gdb/testsuite/gdb.objc/nondebug.m   |  6 +++---
 gdb/testsuite/gdb.objc/objcdecode.m |  6 +++---
 gdb/testsuite/lib/gdb.exp           | 10 +++-------
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/gdb/testsuite/gdb.objc/basicclass.m b/gdb/testsuite/gdb.objc/basicclass.m
index d9b3f23954c..d1d54dfff19 100644
--- a/gdb/testsuite/gdb.objc/basicclass.m
+++ b/gdb/testsuite/gdb.objc/basicclass.m
@@ -1,7 +1,7 @@
 #include <stdio.h>
-#include <objc/Object.h>
+#include <Foundation/NSObject.h>
 
-@interface BasicClass: Object
+@interface BasicClass: NSObject
 {
   id object;
 }
@@ -75,7 +75,7 @@ const char *_NSPrintForDebugger(id object)
 {
   /* This is not really what _NSPrintForDebugger should do, but it
      is a simple test if gdb can call this function */
-  if (object && [object respondsTo: @selector(myDescription)])
+  if (object)
     return [object myDescription];
 
   return NULL;
diff --git a/gdb/testsuite/gdb.objc/nondebug.m b/gdb/testsuite/gdb.objc/nondebug.m
index 352a71c30e3..1ab876443a4 100644
--- a/gdb/testsuite/gdb.objc/nondebug.m
+++ b/gdb/testsuite/gdb.objc/nondebug.m
@@ -1,11 +1,11 @@
 #include <stdio.h>
-#include <objc/Object.h>
+#include <Foundation/NSObject.h>
 
-@interface NonDebug: Object
+@interface NonDebug: NSObject
 {
 }
 @end
-@interface NonDebug2: Object
+@interface NonDebug2: NSObject
 {
 }
 @end
diff --git a/gdb/testsuite/gdb.objc/objcdecode.m b/gdb/testsuite/gdb.objc/objcdecode.m
index a061661ed93..78ff301f3e0 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.m
+++ b/gdb/testsuite/gdb.objc/objcdecode.m
@@ -1,7 +1,7 @@
 #include <stdio.h>
-#include <objc/Object.h>
+#include <Foundation/NSObject.h>
 
-@interface Decode: Object
+@interface Decode: NSObject
 {
 }
 - multipleDef;
@@ -43,7 +43,7 @@ const char *_NSPrintForDebugger(id object)
 {
   /* This is not really what _NSPrintForDebugger should do, but it
      is a simple test if gdb can call this function */
-  if (object && [object respondsTo: @selector(myDescription)])
+  if (object)
     return [object myDescription];
 
   return NULL;
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 856c96771c6..97a9692b4ce 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6884,15 +6884,11 @@ proc gdb_compile_shlib_pthreads {sources dest options} {
 proc gdb_compile_objc {source dest type options} {
     set built_binfile 0
     set why_msg "unrecognized error"
-    foreach lib {-lobjc -lpthreads -lpthread -lthread solaris} {
+    set base_libs {-lgnustep-base -lobjc}
+    foreach lib {-lpthreads -lpthread -lthread "-lpthread -lposix4"} {
         # This kind of wipes out whatever libs the caller may have
         # set.  Or maybe theirs will override ours.  How infelicitous.
-        if { $lib == "solaris" } {
-            set lib "-lpthread -lposix4"
-	}
-        if { $lib != "-lobjc" } {
-	  set lib "-lobjc $lib"
-	}
+	set lib "$base_libs $lib"
         set options_with_lib [concat $options [list libs=$lib quiet]]
         set ccout [gdb_compile $source $dest $type $options_with_lib]
         switch -regexp -- $ccout {
-- 
2.49.0

Re: [PATCH 2/7] Make the gdb.objc tests compile

[Matt Rice]

On Mon, Nov 17, 2025 at 1:33 AM Tom Tromey <tom@tromey.com> wrote:
>
> The gdb.objc tests haven't compiled in years.  This patch fixes this,
> based on a comment in bug 31671.
>
> I don't know whether this approach works with the clang implementation
> of Objective-C.  However, it does work with GCC, provided that
> gnustep-base is installed.

I guess at the time all this stuff broke I never felt that was an option
(and still feel it is terrible, yet better than the status quo)...
Since it is likely *most* people running the testsuite won't have gnustep-base
installed, nor all its shell script environment variables since it
tends to install
its header files in weird locations.

I don't see all those -I and -L flags though, so this seems likely it
depends upon
a certain installation/configuration of gnustep-base which installs
stuff in places
the compiler will pick them up (likely a distro's specific
configuration which isn't
the default to conform to distribution specific installation rules)

But this breakage all happened before the build bots, likely we can
now have an appropriately
configured build bot which will notice all the breakages that are
likely to occur
by people running the testsuite without it installed, so perhaps it is
a more viable approach
now than I imagined it would be then.

> ---
>  gdb/testsuite/gdb.objc/basicclass.m |  6 +++---
>  gdb/testsuite/gdb.objc/nondebug.m   |  6 +++---
>  gdb/testsuite/gdb.objc/objcdecode.m |  6 +++---
>  gdb/testsuite/lib/gdb.exp           | 10 +++-------
>  4 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.objc/basicclass.m b/gdb/testsuite/gdb.objc/basicclass.m
> index d9b3f23954c..d1d54dfff19 100644
> --- a/gdb/testsuite/gdb.objc/basicclass.m
> +++ b/gdb/testsuite/gdb.objc/basicclass.m
> @@ -1,7 +1,7 @@
>  #include <stdio.h>
> -#include <objc/Object.h>
> +#include <Foundation/NSObject.h>
>
> -@interface BasicClass: Object
> +@interface BasicClass: NSObject
>  {
>    id object;
>  }
> @@ -75,7 +75,7 @@ const char *_NSPrintForDebugger(id object)
>  {
>    /* This is not really what _NSPrintForDebugger should do, but it
>       is a simple test if gdb can call this function */
> -  if (object && [object respondsTo: @selector(myDescription)])
> +  if (object)
>      return [object myDescription];
>
>    return NULL;
> diff --git a/gdb/testsuite/gdb.objc/nondebug.m b/gdb/testsuite/gdb.objc/nondebug.m
> index 352a71c30e3..1ab876443a4 100644
> --- a/gdb/testsuite/gdb.objc/nondebug.m
> +++ b/gdb/testsuite/gdb.objc/nondebug.m
> @@ -1,11 +1,11 @@
>  #include <stdio.h>
> -#include <objc/Object.h>
> +#include <Foundation/NSObject.h>
>
> -@interface NonDebug: Object
> +@interface NonDebug: NSObject
>  {
>  }
>  @end
> -@interface NonDebug2: Object
> +@interface NonDebug2: NSObject
>  {
>  }
>  @end
> diff --git a/gdb/testsuite/gdb.objc/objcdecode.m b/gdb/testsuite/gdb.objc/objcdecode.m
> index a061661ed93..78ff301f3e0 100644
> --- a/gdb/testsuite/gdb.objc/objcdecode.m
> +++ b/gdb/testsuite/gdb.objc/objcdecode.m
> @@ -1,7 +1,7 @@
>  #include <stdio.h>
> -#include <objc/Object.h>
> +#include <Foundation/NSObject.h>
>
> -@interface Decode: Object
> +@interface Decode: NSObject
>  {
>  }
>  - multipleDef;
> @@ -43,7 +43,7 @@ const char *_NSPrintForDebugger(id object)
>  {
>    /* This is not really what _NSPrintForDebugger should do, but it
>       is a simple test if gdb can call this function */
> -  if (object && [object respondsTo: @selector(myDescription)])
> +  if (object)
>      return [object myDescription];
>
>    return NULL;
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 856c96771c6..97a9692b4ce 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -6884,15 +6884,11 @@ proc gdb_compile_shlib_pthreads {sources dest options} {
>  proc gdb_compile_objc {source dest type options} {
>      set built_binfile 0
>      set why_msg "unrecognized error"
> -    foreach lib {-lobjc -lpthreads -lpthread -lthread solaris} {
> +    set base_libs {-lgnustep-base -lobjc}
> +    foreach lib {-lpthreads -lpthread -lthread "-lpthread -lposix4"} {
>          # This kind of wipes out whatever libs the caller may have
>          # set.  Or maybe theirs will override ours.  How infelicitous.
> -        if { $lib == "solaris" } {
> -            set lib "-lpthread -lposix4"
> -       }
> -        if { $lib != "-lobjc" } {
> -         set lib "-lobjc $lib"
> -       }
> +       set lib "$base_libs $lib"
>          set options_with_lib [concat $options [list libs=$lib quiet]]
>          set ccout [gdb_compile $source $dest $type $options_with_lib]
>          switch -regexp -- $ccout {
> --
> 2.49.0
>

Re: [PATCH 2/7] Make the gdb.objc tests compile

[Tom Tromey]

Matt> I guess at the time all this stuff broke I never felt that was an
Matt> option (and still feel it is terrible, yet better than the status
Matt> quo)...  Since it is likely *most* people running the testsuite
Matt> won't have gnustep-base installed, nor all its shell script
Matt> environment variables since it tends to install its header files
Matt> in weird locations.

Matt> I don't see all those -I and -L flags though, so this seems likely
Matt> it depends upon a certain installation/configuration of
Matt> gnustep-base which installs stuff in places the compiler will pick
Matt> them up (likely a distro's specific configuration which isn't the
Matt> default to conform to distribution specific installation rules)

Yeah, at least on Fedora, if I "dnf install" the gnustep-base-devel
package, that is enough.

I personally think it's fine to require gdb developers to install random
stuff like this, at least if it is pre-packaged.

Anyway if there's a better way to do it, I'm open to that as well.

Tom

Re: [PATCH 2/7] Make the gdb.objc tests compile

[Matt Rice]

On Tue, Nov 18, 2025 at 12:51 AM Tom Tromey <tom@tromey.com> wrote:
>
> Matt> I guess at the time all this stuff broke I never felt that was an
> Matt> option (and still feel it is terrible, yet better than the status
> Matt> quo)...  Since it is likely *most* people running the testsuite
> Matt> won't have gnustep-base installed, nor all its shell script
> Matt> environment variables since it tends to install its header files
> Matt> in weird locations.
>
> Matt> I don't see all those -I and -L flags though, so this seems likely
> Matt> it depends upon a certain installation/configuration of
> Matt> gnustep-base which installs stuff in places the compiler will pick
> Matt> them up (likely a distro's specific configuration which isn't the
> Matt> default to conform to distribution specific installation rules)
>
> Yeah, at least on Fedora, if I "dnf install" the gnustep-base-devel
> package, that is enough.
>
> I personally think it's fine to require gdb developers to install random
> stuff like this, at least if it is pre-packaged.
>
> Anyway if there's a better way to do it, I'm open to that as well.

I guess the only concrete suggestion I'd have is using the
`gnustep-config --base-libs`, and
`gnustep-config --objc-flags`, for the header files/link flags.

The cpp flags aren't really shown in this patch, I think that should
allow it to work for
the case where someone installs gnustep by hand, using the default
configuration as well
as these distro packages.

Re: [PATCH 2/7] Make the gdb.objc tests compile

[Tom Tromey]

>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

>> Anyway if there's a better way to do it, I'm open to that as well.

Matt> I guess the only concrete suggestion I'd have is using the
Matt> `gnustep-config --base-libs`, and
Matt> `gnustep-config --objc-flags`, for the header files/link flags.

I tried this, and at least on Fedora 40, it doesn't work directly.  The
issue is that apparently some RPM-specific stuff snuck into these files.
So, trying to use them gives me an error about RPM_ARCH not being
defined.

Since my patches are a less invasive change, and help the situation
somewhat, I'm going to go ahead and check them in.  I'll file a bug
about switching to gnustep-config in the future.

Tom

Re: [PATCH 2/7] Make the gdb.objc tests compile

[Matt Rice]

On Tue, Dec 2, 2025 at 4:20 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
>
> >> Anyway if there's a better way to do it, I'm open to that as well.
>
> Matt> I guess the only concrete suggestion I'd have is using the
> Matt> `gnustep-config --base-libs`, and
> Matt> `gnustep-config --objc-flags`, for the header files/link flags.
>
> I tried this, and at least on Fedora 40, it doesn't work directly.  The
> issue is that apparently some RPM-specific stuff snuck into these files.
> So, trying to use them gives me an error about RPM_ARCH not being
> defined.
>
> Since my patches are a less invasive change, and help the situation
> somewhat, I'm going to go ahead and check them in.  I'll file a bug
> about switching to gnustep-config in the future.

Sounds reasonable, thanks for giving it a shot though.
I did have a gander at the fedora 40 srpms and it wasn't obvious where
this could be coming from (no instances of that variable being used
anywhere at all).
mysterious.

[PATCH 3/7] Minor fixes to make gdb.objc tests pass

[Tom Tromey]

Once the gdb.objc tests compile, you can see that they do not pass.
The issue is primarily that they are over-strict about line numbers.
This fixes the problem by replacing these line numbers with $decimal.
---
 gdb/testsuite/gdb.objc/basicclass.exp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.objc/basicclass.exp b/gdb/testsuite/gdb.objc/basicclass.exp
index d1fb64a42de..2be5b2e83ef 100644
--- a/gdb/testsuite/gdb.objc/basicclass.exp
+++ b/gdb/testsuite/gdb.objc/basicclass.exp
@@ -89,36 +89,36 @@ do_objc_tests
 gdb_test "set breakpoint pending off" "" "set breakpoint pending"
 
 gdb_test "break doIt" \
-    "Breakpoint.*at.* file .*$srcfile, line.29.*" \
+    "Breakpoint.*at.* file .*$srcfile, line $decimal.*" \
     "breakpoint method"
 
 gdb_test "break takeArg:" \
-    "Breakpoint.*at.* file .*$srcfile, line.34.*" \
+    "Breakpoint.*at.* file .*$srcfile, line $decimal.*" \
     "breakpoint method with colon"
 
 gdb_test "break newWithArg:" \
-    "Breakpoint.*at.* file .*$srcfile, line.22.*" \
+    "Breakpoint.*at.* file .*$srcfile, line $decimal.*" \
     "breakpoint class method with colon"
 
 #
 # Continue until breakpoint (test re-setting breakpoint)
 #
 gdb_test continue \
-    "Continuing\\..*Breakpoint \[0-9\]+, -.BasicClass takeArg:. \\(self=.*, _cmd=.*, arg=.*\\) at .*$srcfile:34.*" \
+    "Continuing\\..*Breakpoint \[0-9\]+, -.BasicClass takeArg:. \\(self=.*, _cmd=.*, arg=.*\\) at .*$srcfile:$decimal.*" \
     "continue until method breakpoint"
 
 #
 # Test resetting breakpoints when re-running program
 #
 gdb_run_cmd
-gdb_test "" "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:.*" "resetting breakpoints when rerunning"
+gdb_test "" "Breakpoint .+,.*main .*argc.*argv.* at .*$srcfile:.*" "resetting breakpoints when rerunning"
 
 #
 # Continue until breakpoint (test re-setting breakpoint)
 #
 gdb_test continue \
-    "Continuing\\..*Breakpoint \[0-9\]+, -.BasicClass takeArg:. \\(self=.*, _cmd=.*, arg=.*\\) at .*$srcfile:34.*" \
-    "continue until method breakpoint"
+    "Continuing\\..*Breakpoint .+, -.BasicClass takeArg:. \\(self=.*, _cmd=.*, arg=.*\\) at .*$srcfile:$decimal.*" \
+    "continue until method breakpoint again"
 
 #
 # Test printing objects
@@ -139,7 +139,7 @@ gdb_test "print \*self" \
 # Break in a category
 #
 gdb_test "break hiddenMethod" \
-    "Breakpoint.*at.* file .*$srcfile, line.61." \
+    "Breakpoint.*at.* file .*$srcfile, line $decimal." \
     "breakpoint in category method"
 
 
@@ -147,7 +147,7 @@ gdb_test "break hiddenMethod" \
 # Continue until breakpoint (test re-setting category breakpoint)
 #
 gdb_test continue \
-    "Continuing\\..*Breakpoint \[0-9\]+, -.BasicClass\\(Private\\) hiddenMethod. \\(self=.*, _cmd=.*\\) at .*$srcfile:61.*" \
+    "Continuing\\..*Breakpoint \[0-9\]+, -.BasicClass\\(Private\\) hiddenMethod. \\(self=.*, _cmd=.*\\) at .*$srcfile:$decimal.*" \
     "continue until category method"
 
 #
-- 
2.49.0

[PATCH 4/7] Rename lookup_struct_typedef

[Tom Tromey]

The helper function lookup_struct_typedef is only ever called with
"noerr=1".  This patch removes the parameter, changes the name, and
removes some unnecessary code.
---
 gdb/c-exp.y     |  4 ++--
 gdb/objc-lang.c | 31 +++++++------------------------
 gdb/objc-lang.h |  5 ++---
 3 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 9f1cb1b2e2a..6dd2b0800dd 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -3167,8 +3167,8 @@ classify_name (struct parser_state *par_state, const struct block *block,
 	  struct symbol *sym;
 
 	  yylval.theclass.theclass = Class;
-	  sym = lookup_struct_typedef (copy.c_str (),
-				       par_state->expression_context_block, 1);
+	  sym = lookup_struct_noerr (copy.c_str (),
+				     par_state->expression_context_block);
 	  if (sym)
 	    yylval.theclass.type = sym->type ();
 	  return CLASSNAME;
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index b6f42b982eb..b307a0022b5 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -79,31 +79,14 @@ struct objc_method {
 static const registry<objfile>::key<unsigned int> objc_objfile_data;
 
 /* Lookup a structure type named "struct NAME", visible in lexical
-   block BLOCK.  If NOERR is nonzero, return zero if NAME is not
-   suitably defined.  */
+   block BLOCK.  Return nullptr if no such type is found.  */
 
 struct symbol *
-lookup_struct_typedef (const char *name, const struct block *block, int noerr)
+lookup_struct_noerr (const char *name, const struct block *block)
 {
-  struct symbol *sym;
-
-  sym = lookup_symbol (name, block, SEARCH_STRUCT_DOMAIN, 0).symbol;
-
-  if (sym == NULL)
-    {
-      if (noerr)
-	return 0;
-      else
-	error (_("No struct type named %s."), name);
-    }
-  if (sym->type ()->code () != TYPE_CODE_STRUCT)
-    {
-      if (noerr)
-	return 0;
-      else
-	error (_("This context has class, union or enum %s, not a struct."),
-	       name);
-    }
+  symbol *sym = lookup_symbol (name, block, SEARCH_STRUCT_DOMAIN, 0).symbol;
+  if (sym == nullptr || sym->type ()->code () != TYPE_CODE_STRUCT)
+    return nullptr;
   return sym;
 }
 
@@ -212,9 +195,9 @@ value_nsstring (struct gdbarch *gdbarch, const char *ptr, int len)
   else
     error (_("NSString: internal error -- no way to create new NSString"));
 
-  sym = lookup_struct_typedef("NSString", 0, 1);
+  sym = lookup_struct_noerr ("NSString", 0);
   if (sym == NULL)
-    sym = lookup_struct_typedef("NXString", 0, 1);
+    sym = lookup_struct_noerr ("NXString", 0);
   if (sym == NULL)
     type = builtin_type (gdbarch)->builtin_data_ptr;
   else
diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h
index ec6fbe7aaf9..d6a7509f21a 100644
--- a/gdb/objc-lang.h
+++ b/gdb/objc-lang.h
@@ -44,8 +44,7 @@ extern void start_msglist (void);
 extern void add_msglist (struct stoken *str, int addcolon);
 extern int end_msglist (struct parser_state *);
 
-struct symbol *lookup_struct_typedef (const char *name,
-				      const struct block *block,
-				      int noerr);
+struct symbol *lookup_struct_noerr (const char *name,
+				    const struct block *block);
 
 #endif /* GDB_OBJC_LANG_H */
-- 
2.49.0

[PATCH 5/7] Remove a couple Objective-C expression helpers

[Tom Tromey]

The Objective-C expression code has a couple of helper functions with
just a single caller.  This patch unifies them with the appropriate
evaluate method.
---
 gdb/c-exp.h     | 16 ++--------------
 gdb/eval.c      | 13 ++++++-------
 gdb/objc-lang.c | 16 +++++++++++++---
 gdb/objc-lang.h |  3 ---
 4 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/gdb/c-exp.h b/gdb/c-exp.h
index f15185f4da0..8dbbce0f406 100644
--- a/gdb/c-exp.h
+++ b/gdb/c-exp.h
@@ -23,10 +23,6 @@
 #include "expop.h"
 #include "objc-lang.h"
 
-extern struct value *eval_op_objc_selector (struct type *expect_type,
-					    struct expression *exp,
-					    enum noside noside,
-					    const char *sel);
 extern struct value *opencl_value_cast (struct type *type, struct value *arg);
 extern struct value *eval_opencl_assign (struct type *expect_type,
 					 struct expression *exp,
@@ -72,11 +68,7 @@ class objc_nsstring_operation
 
   value *evaluate (struct type *expect_type,
 		   struct expression *exp,
-		   enum noside noside) override
-  {
-    const std::string &str = std::get<0> (m_storage);
-    return value_nsstring (exp->gdbarch, str.c_str (), str.size () + 1);
-  }
+		   enum noside noside) override;
 
   enum exp_opcode opcode () const override
   { return OP_OBJC_NSSTRING; }
@@ -91,11 +83,7 @@ class objc_selector_operation
 
   value *evaluate (struct type *expect_type,
 		   struct expression *exp,
-		   enum noside noside) override
-  {
-    return eval_op_objc_selector (expect_type, exp, noside,
-				  std::get<0> (m_storage).c_str ());
-  }
+		   enum noside noside) override;
 
   enum exp_opcode opcode () const override
   { return OP_OBJC_SELECTOR; }
diff --git a/gdb/eval.c b/gdb/eval.c
index 9aa9665e959..530d5871a9c 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1185,20 +1185,19 @@ ternop_slice_operation::evaluate (struct type *expect_type,
   return value_slice (array, lowbound, upperbound - lowbound + 1);
 }
 
-} /* namespace expr */
-
-/* Helper function that implements the body of OP_OBJC_SELECTOR.  */
-
 struct value *
-eval_op_objc_selector (struct type *expect_type, struct expression *exp,
-		       enum noside noside,
-		       const char *sel)
+objc_selector_operation::evaluate (struct type *expect_type,
+				   struct expression *exp,
+				   enum noside noside)
 {
+  const char *sel = std::get<0> (m_storage).c_str ();
   struct type *selector_type = builtin_type (exp->gdbarch)->builtin_data_ptr;
   return value_from_longest (selector_type,
 			     lookup_child_selector (exp->gdbarch, sel));
 }
 
+} /* namespace expr */
+
 /* A helper function for STRUCTOP_STRUCT.  */
 
 struct value *
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index b307a0022b5..a6abcd60089 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -151,9 +151,17 @@ lookup_child_selector (struct gdbarch *gdbarch, const char *selname)
   return value_as_long (call_function_by_hand (function, NULL, selstring));
 }
 
+namespace expr
+{
+
 struct value *
-value_nsstring (struct gdbarch *gdbarch, const char *ptr, int len)
+objc_nsstring_operation::evaluate (struct type *expect_type,
+				   struct expression *exp,
+				   enum noside noside)
 {
+  const std::string &str = std::get<0> (m_storage);
+  struct gdbarch *gdbarch = exp->gdbarch;
+
   struct type *char_type = builtin_type (gdbarch)->builtin_char;
   struct value *stringValue[3];
   struct value *function, *nsstringValue;
@@ -163,8 +171,8 @@ value_nsstring (struct gdbarch *gdbarch, const char *ptr, int len)
   if (!target_has_execution ())
     return 0;		/* Can't call into inferior to create NSString.  */
 
-  stringValue[2] = value_string(ptr, len, char_type);
-  stringValue[2] = value_coerce_array(stringValue[2]);
+  stringValue[2] = value_string (str.c_str (), str.size () + 1, char_type);
+  stringValue[2] = value_coerce_array (stringValue[2]);
   /* _NSNewStringFromCString replaces "istr" after Lantern2A.  */
   if (lookup_minimal_symbol (current_program_space,
 			     "_NSNewStringFromCString").minsym != nullptr)
@@ -207,6 +215,8 @@ value_nsstring (struct gdbarch *gdbarch, const char *ptr, int len)
   return nsstringValue;
 }
 
+} /* namespace expr */
+
 /* Class representing the Objective-C language.  */
 
 class objc_language : public language_defn
diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h
index d6a7509f21a..a472556d019 100644
--- a/gdb/objc-lang.h
+++ b/gdb/objc-lang.h
@@ -36,9 +36,6 @@ extern int find_objc_msgcall (CORE_ADDR pc, CORE_ADDR *new_pc);
 extern const char *find_imps (const char *method,
 			      std::vector<const char *> *symbol_names);
 
-extern struct value *value_nsstring (struct gdbarch *gdbarch,
-				     const char *ptr, int len);
-
 /* for parsing Objective C */
 extern void start_msglist (void);
 extern void add_msglist (struct stoken *str, int addcolon);
-- 
2.49.0

[PATCH 6/7] Avoid crash with "NSString" literals

[Tom Tromey]

Evaluating an Objective-C "NSString" literal will cause gdb to crash.
This patch fixes the crash.

I think the result here still isn't correct -- I see a warning from
the runtime ("autorelease called without pool for object") with the
new code.

However, not crashing is an improvement on its own.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20501
---
 gdb/c-exp.y                      | 10 +++++-----
 gdb/objc-lang.c                  |  3 ++-
 gdb/testsuite/gdb.objc/print.exp |  4 ++++
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 6dd2b0800dd..fadd735c517 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -223,7 +223,7 @@ static void c_print_token (FILE *file, int type, YYSTYPE value);
    nonterminal "name", which matches either NAME or TYPENAME.  */
 
 %token <tsval> STRING
-%token <sval> NSSTRING		/* ObjC Foundation "NSString" literal */
+%token <tsval> NSSTRING		/* ObjC Foundation "NSString" literal */
 %token SELECTOR			/* ObjC "@selector" pseudo-operator   */
 %token <tsval> CHAR
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
@@ -1030,12 +1030,12 @@ exp	:	string_exp
 			}
 	;
 
-exp     :	NSSTRING	/* ObjC NextStep NSString constant
-				 * of the form '@' '"' string '"'.
-				 */
+exp     :	NSSTRING
 			{
+			  /* ObjC NextStep NSString constant of the
+			     form '@' '"' string '"'.  */
 			  pstate->push_new<objc_nsstring_operation>
-			    (copy_name ($1));
+			    (std::string ($1.ptr, $1.length));
 			}
 	;
 
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index a6abcd60089..9804f2c3963 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -169,7 +169,8 @@ objc_nsstring_operation::evaluate (struct type *expect_type,
   struct type *type;
 
   if (!target_has_execution ())
-    return 0;		/* Can't call into inferior to create NSString.  */
+    error (_("evaluation of this expression "
+	     "requires the target program to be active"));
 
   stringValue[2] = value_string (str.c_str (), str.size () + 1, char_type);
   stringValue[2] = value_coerce_array (stringValue[2]);
diff --git a/gdb/testsuite/gdb.objc/print.exp b/gdb/testsuite/gdb.objc/print.exp
index e3f4177d997..2e9fee3b869 100644
--- a/gdb/testsuite/gdb.objc/print.exp
+++ b/gdb/testsuite/gdb.objc/print.exp
@@ -62,6 +62,10 @@ clean_restart
 if { [set_lang_objc] } {
     test_float_accepted
     test_float_rejected
+
+    gdb_test {print @"hi"} \
+	"evaluation of this expression requires the target program to be active" \
+	"simple nsstring"
 } else {
     warning "Objective-c print tests suppressed"
 }
-- 
2.49.0

[PATCH 7/7] Rewrite the @selector code

[Tom Tromey]

This rewrites the Objective-C @selector implementation in c-exp.y,
following a couple general comments in the patch supplied for
bug 20503.

I suspect something else changed in the Objective-C runtime, though,
as most of the examples in that bug still do not work, even though
this series (I believe) addresses all the same points.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20503
---
 gdb/c-exp.y | 53 +++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 8 deletions(-)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index fadd735c517..829058027f3 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -224,7 +224,7 @@ static void c_print_token (FILE *file, int type, YYSTYPE value);
 
 %token <tsval> STRING
 %token <tsval> NSSTRING		/* ObjC Foundation "NSString" literal */
-%token SELECTOR			/* ObjC "@selector" pseudo-operator   */
+%token <sval> SELECTOR		/* ObjC "@selector" pseudo-operator   */
 %token <tsval> CHAR
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
 %token <ssym> UNKNOWN_CPP_NAME
@@ -918,10 +918,10 @@ exp	:	DOLLAR_VARIABLE
 			}
 	;
 
-exp	:	SELECTOR '(' name ')'
+exp	:	SELECTOR
 			{
 			  pstate->push_new<objc_selector_operation>
-			    (copy_name ($3));
+			    (copy_name ($1));
 			}
 	;
 
@@ -2664,6 +2664,43 @@ static bool last_was_structop;
 /* Depth of parentheses.  */
 static int paren_depth;
 
+/* Lex an Objective-C @selector.  Return true if lexed.  In this case,
+   sets the resulting token and updates the lex pointer.  Otherwise
+   returns false and updates nothing.  */
+
+static bool
+lex_selector (const char **lex_ptr, struct stoken *token)
+{
+  const char *p = *lex_ptr;
+
+  if (!startswith (p, "selector"))
+    return false;
+
+  p += strlen ("selector");
+  p = skip_spaces (p);
+  if (*p != '(')
+    return false;
+  ++p;
+
+  /* The selector name matches [A-Za-z0-9:_-]+.  We could probably be
+     a bit more refined but meh.  */
+  const char *start = p;
+  while (c_isalnum (*p) || *p == ':' || *p == '_' || *p == '-')
+    ++p;
+  if (p == start)
+    return false;
+  const char *end = p;
+
+  p = skip_spaces (p);
+  if (*p != ')')
+    return false;
+  ++p;
+
+  *lex_ptr = p;
+  *token = { start, (int) (end - start) };
+  return true;
+}
+
 /* Read one token, getting characters through lexptr.  */
 
 static int
@@ -2872,12 +2909,11 @@ lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
 
 	if (par_state->language ()->la_language == language_objc)
 	  {
-	    size_t len = strlen ("selector");
-
-	    if (strncmp (p, "selector", len) == 0
-		&& (p[len] == '\0' || c_isspace (p[len])))
+	    struct stoken sel_token;
+	    if (lex_selector (&p, &sel_token))
 	      {
-		pstate->lexptr = p + len;
+		pstate->lexptr = p;
+		yylval.sval = sel_token;
 		return SELECTOR;
 	      }
 	    else if (*p == '"')
@@ -3556,6 +3592,7 @@ c_print_token (FILE *file, int type, YYSTYPE value)
 
     case NSSTRING:
     case DOLLAR_VARIABLE:
+    case SELECTOR:
       parser_fprintf (file, "sval<%s>", copy_name (value.sval).c_str ());
       break;
 
-- 
2.49.0