Re: STatus of MPS branch

[Helmut Eller]

On Sun, Apr 21 2024, Gerd Möllmann wrote:

[..]
>> igc.c:229:18: warning: ‘pvec_type’ is narrower than values of its type
>>   229 |   enum pvec_type pvec_type : IGC_PVEC_BITS;
>
> Good catch, clang didn't find that. You could increase the bit field
> size by 1, and the one for the hash by 1, for example.

I did this; GCC stopped complaining but otherwise it made no observable
difference.

[..]
>>  # define igc_assert(expr)                         \
>>     if (!(expr))                                   \
>>       igc_assert_fail (__FILE__, __LINE__, #expr); \
>> -   else
>> +   /* else */
[..]
> Don't know, this looks a bit problemantic, but if it's the problem I
> don't know. Removing the else means something like
>
>   if (x)
>     igc_assert (...);
>   else
>     do something else
>
> expand to something one doesn't want. The else is swallowed by the if in
> the macro...

Without some change GCC emits lots of warnings of this kind:
igc.c: In function ‘to_words’:
igc.c:246:49: warning: suggest braces around empty body in an ‘else’ statement 
[-Wempty-body]
  246 |   igc_assert (nbytes % sizeof (mps_word_t) == 0);
      |                                                 ^

So maybe the macro should end with "else {}".

[..]
>>  static mps_res_t
>>  fix_image (mps_ss_t ss, struct image *i)
>>  {
>> +#ifdef HAVE_WINDOW_SYSTEM
>>    MPS_SCAN_BEGIN (ss)
>>    {
>>      IGC_FIX12_OBJ (ss, &i->spec);
>> @@ -964,6 +966,7 @@ fix_image (mps_ss_t ss, struct image *i)
>>    }
>>    MPS_SCAN_END (ss);
>>    return MPS_RES_OK;
>> +#endif
>>  }
>
> If I'm reading this one right, the function doesn't return a value.
> But maybe this doesn't get compiled?

Yes. it's just to get it compiled; struct image isn't defined with my
config.

>>   INFO     Scraping files for loaddefs...80% 
>>
>> Program received signal SIGSEGV, Segmentation fault.
>>
>> Breakpoint 1, handle_sigsegv (sig=11, 
>>     siginfo=0x555555f6ed70 <sigsegv_stack+64784>, 
>>     arg=0x555555f6ec40 <sigsegv_stack+64480>) at sysdep.c:1930
>> 1930      bool fatal = gc_in_progress;
>> (gdb) backtrace 
>> #0  handle_sigsegv (sig=11, siginfo=0x555555f6ed70 <sigsegv_stack+64784>, 
>>     arg=0x555555f6ec40 <sigsegv_stack+64480>) at sysdep.c:1930
>> #1  <signal handler called>
>> #2  string_intervals (s=XIL(0x7ffff0698edc))
>>     at /scratch/emacs/emacs-igc/src/lisp.h:4063
>> #3  0x0000555555767d97 in concat_to_string (nargs=3, args=0x7fffe5f035b8)
>>     at fns.c:957
>
> I think I'd start in #2 here, by looking at S. First question would be
> if the string itself it ok, and what kind of string it is. I haven't
> used GDB with Emacs here for a very long time (no GDB on macOS), so I'm
> a bit out of my comfort zone.
>
> # We need the pointer to the Lisp_String that's in S, let's say it's P.
> # Don't know how, sorry
> ???

I changed some things and now I can no longer reproduce the exact same
problem.  But it looked to me like a perfectly normal string with
intervals=0x0.  Strangely, accessing the intervals field from GDB didn't
cause any SIGSEGV.

However, after setting breakpoints in handle_sigsegv and sigHandle (from
MPS) I discovered that sigHandle wasn't called.  So my hypothesis is
that the signal handler isn't initialized properly.  In particular, it
seems problematic that ProtSetup (from MPS) is called before
init_signals.

Then I moved the call to init_signals in emacs.c up before the call to
init_igc and voilà: the build completed.  With an apparently working
(tty-only) Emacs.

Helmut