[RFA/commit(Ada)] SEGV during symbol completion - VEC usage issue

[RFA/commit(Ada)] SEGV during symbol completion - VEC usage issue

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

It's weird that I didn't spot this problem earlier: When the number
of possible completions exceeds 128 (the initial size of the VEC
we use to store all completions), the debugger segfaults.

The problem, I think, is that I didn't understand how VEC_safe_push
works underneath. I didn't realize that this routine would change
the value of the given VEC*, I thought it would only change the
underlying record.

I used to have:

    procedure bla (VEC(char_ptr) *sv, ...)
    {
      ...
      VEC_safe_push (char_ptr, sv, some_string);
    }

When SV had to be expanded, the value of SV ended up being changed,
but this new address was never propagated back to the caller.

So I fixed the problem by turning parameter SV into a VEC**.

2008-02-07  Joel Brobecker  <brobecker@adacore.com>

        * ada-lang.c (symbol_completion_add): Make SV parameter a VEC**
        instead of just a VEC*. Update use of SV.
        (ada_make_symbol_completion_list): Update symbol_completion_add calls.

Tested on x86-linux, no regression. I'll commit now because it cures a SEGV
that is easy to hit, but I'd appreciate a second pair of eyes...

Thanks,
-- 
Joel

[-- Attachment #2: VEC.diff --]
[-- Type: text/plain, Size: 3106 bytes --]

Index: ada-lang.c
===================================================================
--- ada-lang.c	(revision 167)
+++ ada-lang.c	(revision 168)
@@ -5498,7 +5498,7 @@ DEF_VEC_P (char_ptr);
    encoded).  */
 
 static void
-symbol_completion_add (VEC(char_ptr) *sv,
+symbol_completion_add (VEC(char_ptr) **sv,
                        const char *sym_name,
                        const char *text, int text_len,
                        const char *orig_text, const char *word,
@@ -5534,7 +5534,7 @@ symbol_completion_add (VEC(char_ptr) *sv
       strcat (completion, match);
     }
 
-  VEC_safe_push (char_ptr, sv, completion);
+  VEC_safe_push (char_ptr, *sv, completion);
 }
 
 /* Return a list of possible symbol names completing TEXT0.  The list
@@ -5597,7 +5597,7 @@ ada_make_symbol_completion_list (char *t
                  + ps->n_global_syms); psym++)
       {
         QUIT;
-        symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (*psym),
+        symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (*psym),
                                text, text_len, text0, word,
                                wild_match, encoded);
       }
@@ -5607,7 +5607,7 @@ ada_make_symbol_completion_list (char *t
                  + ps->n_static_syms); psym++)
       {
         QUIT;
-        symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (*psym),
+        symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (*psym),
                                text, text_len, text0, word,
                                wild_match, encoded);
       }
@@ -5621,7 +5621,7 @@ ada_make_symbol_completion_list (char *t
   ALL_MSYMBOLS (objfile, msymbol)
   {
     QUIT;
-    symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (msymbol),
+    symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (msymbol),
                            text, text_len, text0, word, wild_match, encoded);
   }
 
@@ -5635,7 +5635,7 @@ ada_make_symbol_completion_list (char *t
 
       ALL_BLOCK_SYMBOLS (b, iter, sym)
       {
-        symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (sym),
+        symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
                                text, text_len, text0, word,
                                wild_match, encoded);
       }
@@ -5650,7 +5650,7 @@ ada_make_symbol_completion_list (char *t
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (sym),
+      symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
                              text, text_len, text0, word,
                              wild_match, encoded);
     }
@@ -5665,7 +5665,7 @@ ada_make_symbol_completion_list (char *t
       continue;
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (sym),
+      symbol_completion_add (&completions, SYMBOL_LINKAGE_NAME (sym),
                              text, text_len, text0, word,
                              wild_match, encoded);
     }

Re: [RFA/commit(Ada)] SEGV during symbol completion - VEC usage issue

[Daniel Jacobowitz]

On Thu, Feb 07, 2008 at 02:24:59PM -0800, Joel Brobecker wrote:
> So I fixed the problem by turning parameter SV into a VEC**.
> 
> 2008-02-07  Joel Brobecker  <brobecker@adacore.com>
> 
>         * ada-lang.c (symbol_completion_add): Make SV parameter a VEC**
>         instead of just a VEC*. Update use of SV.
>         (ada_make_symbol_completion_list): Update symbol_completion_add calls.
> 
> Tested on x86-linux, no regression. I'll commit now because it cures a SEGV
> that is easy to hit, but I'd appreciate a second pair of eyes...

Looks correct to me also.

-- 
Daniel Jacobowitz
CodeSourcery