building/using address-sanitizer-enabled emacs?

building/using address-sanitizer-enabled emacs?

[Jim Meyering]

Has anyone managed to dump an ASAN-enabled emacs recently?
I can build and use an ASAN-enabled temacs, but it's too slow, of course.
When I build as follows, (using latest gcc-built-from-today's-git[*] --
very recent gcc is required for my use of the new
-fsanitize-address-use-after-scope), the temacs-to-emacs dump fails
with a global-buffer-overflow:

  san='-fsanitize-address-use-after-scope -fsanitize=address -static-libasan'
  ./configure --prefix=/p/p/emacs-asan --without-gpm --without-x \
    --with-x-toolkit=no --with-png=no --with-jpeg=no --with-sound=no \
    CFLAGS="-O0 -ggdb3 $san" LDFLAGS="$san" && make

I guess it's not too surprising -- given what dumping does -- that it
is not yet ASAN-aware, but there are so many traces of address sanitizer
work already in emacs that I'm hoping someone has already dealt with this.

------------------
Finding pointers to doc strings...
Finding pointers to doc strings...done
Dumping under the name emacs
=================================================================
==8192==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000001d561e1 at pc 0x000000463102 bp 0x7fffffffbca0 sp 0x7fffffffb450
READ of size 13643296 at 0x000001d561e1 thread T0
    #0 0x463101 in __interceptor_memcpy /h/j/gcc/libsanitizer/asan/asan_interceptors.cc:456
    #1 0x9896c0 in unexec /h/j/emacs/src/unexelf.c:407
    #2 0x74d8ad in Fdump_emacs /h/j/emacs/src/emacs.c:2191
    #3 0x8df69d in eval_sub /h/j/emacs/src/eval.c:2223
    #4 0x8d51c4 in Fprogn /h/j/emacs/src/eval.c:449
    #5 0x8deed5 in eval_sub /h/j/emacs/src/eval.c:2173
    #6 0x8d4d41 in Fif /h/j/emacs/src/eval.c:406
    #7 0x8deed5 in eval_sub /h/j/emacs/src/eval.c:2173
    #8 0x945a9c in readevalloop /h/j/emacs/src/lread.c:1947
    #9 0x942b1d in Fload /h/j/emacs/src/lread.c:1352
    #10 0x8df946 in eval_sub /h/j/emacs/src/eval.c:2234
    #11 0x8ddf54 in Feval /h/j/emacs/src/eval.c:2042
    #12 0x751a34 in top_level_2 /h/j/emacs/src/keyboard.c:1121
    #13 0x8da12d in internal_condition_case /h/j/emacs/src/eval.c:1326
    #14 0x751a97 in top_level_1 /h/j/emacs/src/keyboard.c:1129
    #15 0x8d8911 in internal_catch /h/j/emacs/src/eval.c:1091
    #16 0x751899 in command_loop /h/j/emacs/src/keyboard.c:1090
    #17 0x75033f in recursive_edit_1 /h/j/emacs/src/keyboard.c:697
    #18 0x7506dd in Frecursive_edit /h/j/emacs/src/keyboard.c:768
    #19 0x74bbb9 in main /h/j/emacs/src/emacs.c:1687
    #20 0x7ffff5b52400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    #21 0x40d369 in _start (/h/j/emacs/src/temacs+0x40d369)

0x000001d561e1 is located 0 bytes to the right of global variable 'display_completed' defined in 'dispnew.c:100:6' (0x1d561e0) of size 1
  'display_completed' is ascii string ''
0x000001d561e1 is located 63 bytes to the left of global variable 'delayed_size_change' defined in 'dispnew.c:104:13' (0x1d56220) of size 1
SUMMARY: AddressSanitizer: global-buffer-overflow /h/j/gcc/libsanitizer/asan/asan_interceptors.cc:456 in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x0000803a2be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000803a2bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000803a2c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000803a2c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000803a2c20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0000803a2c30: 00 00 00 00 00 00 00 00 00 00 00 00[01]f9 f9 f9
  0x0000803a2c40: f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0000803a2c50: 00 00 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x0000803a2c60: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x0000803a2c70: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x0000803a2c80: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==8192==ABORTING
Makefile:735: recipe for target 'bootstrap-emacs' failed
make[1]: *** [bootstrap-emacs] Error 1
make[1]: Leaving directory '/h/j/emacs/src'
Makefile:416: recipe for target 'src' failed
make: *** [src] Error 2

-------------------
[*] One caveat: to get past a gcc ICE (just reported as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80659), I had to apply this kludgey patch:

diff --git a/src/process.c b/src/process.c
index 0edd092..8abd0d2 100644
--- a/src/process.c
+++ b/src/process.c
@@ -4724,10 +4725,13 @@ server_accept_connection (Lisp_Object server, int channel)
     case AF_LOCAL:
 #endif
     default:
+      abort ();
+#if 0
       caller = Fnumber_to_string (make_number (connect_counter));
       AUTO_STRING (space_less_than, " <");
       AUTO_STRING (greater_than, ">");
       caller = concat3 (space_less_than, caller, greater_than);
+#endif
       break;
     }

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

A while ago I got address sanitization to work with temacs, but gave up on 
dumped Emacs because it was too much of a pain. Since we already need to get 
Emacs to work without dumping for other reasons, I put the address-sanitization 
project on the back burner, figuring that it wasn't worth my time if a dump-free 
Emacs will be practical soon.

The project to get Emacs to work without dumping has run into problems, 
unfortunately. The currently-favored technical approach (see the 
scratch/raeburn-startup branch) isn't fully working and apparently will slow 
down Emacs startup significantly, which makes it hard to be enthusiastic about 
it. Progress has been slow: no changes have been installed since April 10, and 
the last message from Ken Raeburn, its principal hacker, was April 16 and began 
"This is probably going to make my brain hurt" 
<http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00468.html>.

Daniel Colascione proposed an alternative approach in 
<http://lists.gnu.org/archive/html/emacs-devel/2016-11/msg00625.html> but as he 
noted in <http://lists.gnu.org/archive/html/emacs-devel/2016-12/msg00861.html> 
it has some problems with weak hash tables and we haven't heard from Daniel 
since January, possibly because he was discouraged by Eli's negative reaction to 
this approach.

Re: building/using address-sanitizer-enabled emacs?

[Jim Meyering]

On Sun, May 7, 2017 at 12:54 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> A while ago I got address sanitization to work with temacs, but gave up on
> dumped Emacs because it was too much of a pain. Since we already need to get
> Emacs to work without dumping for other reasons, I put the
> address-sanitization project on the back burner, figuring that it wasn't
> worth my time if a dump-free Emacs will be practical soon.
>
> The project to get Emacs to work without dumping has run into problems,
> unfortunately. The currently-favored technical approach (see the
> scratch/raeburn-startup branch) isn't fully working and apparently will slow
> down Emacs startup significantly, which makes it hard to be enthusiastic
> about it. Progress has been slow: no changes have been installed since April
> 10, and the last message from Ken Raeburn, its principal hacker, was April
> 16 and began "This is probably going to make my brain hurt"
> <http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00468.html>.
>
> Daniel Colascione proposed an alternative approach in
> <http://lists.gnu.org/archive/html/emacs-devel/2016-11/msg00625.html> but as
> he noted in
> <http://lists.gnu.org/archive/html/emacs-devel/2016-12/msg00861.html> it has
> some problems with weak hash tables and we haven't heard from Daniel since
> January, possibly because he was discouraged by Eli's negative reaction to
> this approach.

Darn. Thanks for the back-story.

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 7 May 2017 12:54:53 -0700
> 
> The project to get Emacs to work without dumping has run into problems, 
> unfortunately. The currently-favored technical approach (see the 
> scratch/raeburn-startup branch) isn't fully working and apparently will slow 
> down Emacs startup significantly, which makes it hard to be enthusiastic about 
> it. Progress has been slow: no changes have been installed since April 10, and 
> the last message from Ken Raeburn, its principal hacker, was April 16 and began 
> "This is probably going to make my brain hurt" 
> <http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00468.html>.

From my POV, Ken's branch is just waiting a decision to be merged onto
master.  I'm not aware of any problems with its current state, but I
have no doubt that problems will start popping once we do merge it and
start using it seriously.  (Any other similar solution will most
probably bump into the same or very similar problems.)

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

Eli Zaretskii wrote:
> I'm not aware of any problems with its current state

Startup performance is the main problem that we were worried about for that 
approach, and Ken's most recent message on the topic said that startup 
performance remains a significant issue.

http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00059.html

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> Cc: jim@meyering.net, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 7 May 2017 22:42:22 -0700
> 
> Eli Zaretskii wrote:
> > I'm not aware of any problems with its current state
> 
> Startup performance is the main problem that we were worried about for that 
> approach, and Ken's most recent message on the topic said that startup 
> performance remains a significant issue.

That's not a problem (as in "bug", which were fixed by Ken, at least
those that we know about).  This "problem" is a limitation of the
method, and I'm not sure the method implemented on the branch can be
significantly sped up, unless we completely rethink what Lisp we load
at startup and how.

This limitation is exactly the reason why we will soon have to decide
whether we merge the branch and start using it, improving what can be
improved and living with that which cannot, or keep using unexec at
least till Emacs 27.

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

On 05/08/2017 07:39 AM, Eli Zaretskii wrote:
>> Eli Zaretskii wrote:
>>> I'm not aware of any problems with its current state
>> Startup performance is the main problem that we were worried about for that
>> approach, and Ken's most recent message on the topic said that startup
>> performance remains a significant issue.
> That's not a problem (as in "bug"

... which is why I wrote "problem" and not "bug". Performance problems 
can be significant issues.

Of course this would not be the only performance problem in master. 
Still, it would make Emacs performance worse and as I understand it this 
is the main reason this approach has not been incorporated thus far.

> This limitation is exactly the reason why we will soon have to decide
> whether we merge the branch and start using it, improving what can be
> improved and living with that which cannot, or keep using unexec
Those are not the only two choices available to us.

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> Cc: jim@meyering.net, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 8 May 2017 07:46:43 -0700
> 
> Still, it would make Emacs performance worse and as I understand it this 
> is the main reason this approach has not been incorporated thus far.

No, the reason is that the decision has not yet been made.

> > This limitation is exactly the reason why we will soon have to decide
> > whether we merge the branch and start using it, improving what can be
> > improved and living with that which cannot, or keep using unexec
> Those are not the only two choices available to us.

What are the alternatives?

Re: building/using address-sanitizer-enabled emacs?

[Jim Meyering]

On Mon, May 8, 2017 at 9:04 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Cc: jim@meyering.net, emacs-devel@gnu.org
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Date: Mon, 8 May 2017 07:46:43 -0700
>>
>> Still, it would make Emacs performance worse and as I understand it this
>> is the main reason this approach has not been incorporated thus far.
>
> No, the reason is that the decision has not yet been made.
>
>> > This limitation is exactly the reason why we will soon have to decide
>> > whether we merge the branch and start using it, improving what can be
>> > improved and living with that which cannot, or keep using unexec
>> Those are not the only two choices available to us.
>
> What are the alternatives?

Hoping to make some progress, I built src/temacs with the kludgey
patch mentioned above (to work around the gcc7/8 ICE)

  san='-fsanitize-address-use-after-scope -fsanitize=address
-static-libasan'; ./configure --without-gpm --without-x
--with-x-toolkit=no --with-png=no --with-jpeg=no --with-sound=no
CFLAGS="-O0 -ggdb3 $san" LDFLAGS="$san" && make

[note, that the build failed due to the aforementioned global buffer
overrun while dumping, but it did succeed in building src/temacs]
and tried to use it on a foo.gpg file like this:

  echo foo |gpg -c > foo.gpg
  src/temacs -q foo.gpg 2> err

which reported this stack buffer overrun:

==24522==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7ffd5cc4d928 at pc 0x00000073fd76 bp 0x7ffd5cc4d910 sp
0x7ffd5cc4d908
READ of size 8 at 0x7ffd5cc4d928 thread T0
    #0 0x73fd75 in PSEUDOVECTORP /home/j/w/co/emacs/src/lisp.h:1454
    #1 0x7438c9 in BUFFERP /home/j/w/co/emacs/src/buffer.h:887
    #2 0x9c64b6 in call_process /home/j/w/co/emacs/src/callproc.c:702
    #3 0x9c41db in Fcall_process /home/j/w/co/emacs/src/callproc.c:270
    #4 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
    #5 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
    #6 0x8e072d in Fapply /home/j/w/co/emacs/src/eval.c:2375
    #7 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
    #8 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
    #9 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #10 0x8e4b55 in funcall_lambda /home/j/w/co/emacs/src/eval.c:3022
    #11 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #12 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #13 0x8e4b55 in funcall_lambda /home/j/w/co/emacs/src/eval.c:3022
    #14 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #15 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #16 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #17 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #18 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #19 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #20 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #21 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #22 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #23 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #24 0x8e072d in Fapply /home/j/w/co/emacs/src/eval.c:2375
    #25 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
    #26 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
    #27 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #28 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #29 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #30 0x8e1f8d in call6 /home/j/w/co/emacs/src/eval.c:2649
    #31 0x8034eb in Finsert_file_contents /home/j/w/co/emacs/src/fileio.c:3602
    #32 0x8e367f in funcall_subr /home/j/w/co/emacs/src/eval.c:2831
    #33 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
    #34 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #35 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #36 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #37 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #38 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #39 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #40 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #41 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #42 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #43 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #44 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #45 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #46 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #47 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #48 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
    #49 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
    #50 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
    #51 0x8e3f96 in apply_lambda /home/j/w/co/emacs/src/eval.c:2881
    #52 0x8df8ab in eval_sub /home/j/w/co/emacs/src/eval.c:2265
    #53 0x8dda2c in Feval /home/j/w/co/emacs/src/eval.c:2042
    #54 0x8df175 in eval_sub /home/j/w/co/emacs/src/eval.c:2223
    #55 0x945574 in readevalloop /home/j/w/co/emacs/src/lread.c:1947
    #56 0x9425f5 in Fload /home/j/w/co/emacs/src/lread.c:1352
    #57 0x8df41e in eval_sub /home/j/w/co/emacs/src/eval.c:2234
    #58 0x8dda2c in Feval /home/j/w/co/emacs/src/eval.c:2042
    #59 0x751a34 in top_level_2 /home/j/w/co/emacs/src/keyboard.c:1121
    #60 0x8d9c05 in internal_condition_case /home/j/w/co/emacs/src/eval.c:1326
    #61 0x751a97 in top_level_1 /home/j/w/co/emacs/src/keyboard.c:1129
    #62 0x8d83e9 in internal_catch /home/j/w/co/emacs/src/eval.c:1091
    #63 0x751899 in command_loop /home/j/w/co/emacs/src/keyboard.c:1090
    #64 0x75033f in recursive_edit_1 /home/j/w/co/emacs/src/keyboard.c:697
    #65 0x7506dd in Frecursive_edit /home/j/w/co/emacs/src/keyboard.c:768
    #66 0x74bbb9 in main /home/j/w/co/emacs/src/emacs.c:1687
    #67 0x7f40c7732400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    #68 0x40d369 in _start (/home/j/w/co/emacs/src/temacs+0x40d369)

Address 0x7ffd5cc4d928 is located in stack of thread T0 at offset 136 in frame
    #0 0x9c8c15 in child_setup /home/j/w/co/emacs/src/callproc.c:1179

  This frame has 2 object(s):
    [32, 40) 'display'
    [96, 104) 'tmp' <== Memory access at offset 136 overflows this variable
HINT: this may be a false positive if your program uses some custom
stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow
/home/j/w/co/emacs/src/lisp.h:1454 in PSEUDOVECTORP
Shadow bytes around the buggy address:
  0x10002b981ad0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981ae0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981af0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981b10: 00 00 00 00 f1 f1 f1 f1 00 f2 f2 f2 f2 f2 f2 f2
=>0x10002b981b20: 00 f2 f2 f2 f3[f3]f3 f3 00 00 00 00 00 00 00 00
  0x10002b981b30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002b981b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==24522==ABORTING

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> From: Jim Meyering <jim@meyering.net>
> Date: Mon, 8 May 2017 22:48:06 -0700
> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel <emacs-devel@gnu.org>
> 
>   echo foo |gpg -c > foo.gpg
>   src/temacs -q foo.gpg 2> err
> 
> which reported this stack buffer overrun:
> 
> ==24522==ERROR: AddressSanitizer: stack-buffer-overflow on address
> 0x7ffd5cc4d928 at pc 0x00000073fd76 bp 0x7ffd5cc4d910 sp
> 0x7ffd5cc4d908
> READ of size 8 at 0x7ffd5cc4d928 thread T0
>     #0 0x73fd75 in PSEUDOVECTORP /home/j/w/co/emacs/src/lisp.h:1454
>     #1 0x7438c9 in BUFFERP /home/j/w/co/emacs/src/buffer.h:887
>     #2 0x9c64b6 in call_process /home/j/w/co/emacs/src/callproc.c:702
>     #3 0x9c41db in Fcall_process /home/j/w/co/emacs/src/callproc.c:270
>     #4 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
>     #5 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>     #6 0x8e072d in Fapply /home/j/w/co/emacs/src/eval.c:2375
>     #7 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
>     #8 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>     #9 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #10 0x8e4b55 in funcall_lambda /home/j/w/co/emacs/src/eval.c:3022
>     #11 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #12 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #13 0x8e4b55 in funcall_lambda /home/j/w/co/emacs/src/eval.c:3022
>     #14 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #15 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #16 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #17 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #18 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #19 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #20 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #21 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #22 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #23 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #24 0x8e072d in Fapply /home/j/w/co/emacs/src/eval.c:2375
>     #25 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
>     #26 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>     #27 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #28 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #29 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #30 0x8e1f8d in call6 /home/j/w/co/emacs/src/eval.c:2649
>     #31 0x8034eb in Finsert_file_contents /home/j/w/co/emacs/src/fileio.c:3602
>     #32 0x8e367f in funcall_subr /home/j/w/co/emacs/src/eval.c:2831
>     #33 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>     #34 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #35 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #36 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #37 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #38 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #39 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #40 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #41 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #42 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #43 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #44 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #45 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #46 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #47 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #48 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>     #49 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>     #50 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>     #51 0x8e3f96 in apply_lambda /home/j/w/co/emacs/src/eval.c:2881
>     #52 0x8df8ab in eval_sub /home/j/w/co/emacs/src/eval.c:2265
>     #53 0x8dda2c in Feval /home/j/w/co/emacs/src/eval.c:2042
>     #54 0x8df175 in eval_sub /home/j/w/co/emacs/src/eval.c:2223
>     #55 0x945574 in readevalloop /home/j/w/co/emacs/src/lread.c:1947
>     #56 0x9425f5 in Fload /home/j/w/co/emacs/src/lread.c:1352
>     #57 0x8df41e in eval_sub /home/j/w/co/emacs/src/eval.c:2234
>     #58 0x8dda2c in Feval /home/j/w/co/emacs/src/eval.c:2042
>     #59 0x751a34 in top_level_2 /home/j/w/co/emacs/src/keyboard.c:1121
>     #60 0x8d9c05 in internal_condition_case /home/j/w/co/emacs/src/eval.c:1326
>     #61 0x751a97 in top_level_1 /home/j/w/co/emacs/src/keyboard.c:1129
>     #62 0x8d83e9 in internal_catch /home/j/w/co/emacs/src/eval.c:1091
>     #63 0x751899 in command_loop /home/j/w/co/emacs/src/keyboard.c:1090
>     #64 0x75033f in recursive_edit_1 /home/j/w/co/emacs/src/keyboard.c:697
>     #65 0x7506dd in Frecursive_edit /home/j/w/co/emacs/src/keyboard.c:768
>     #66 0x74bbb9 in main /home/j/w/co/emacs/src/emacs.c:1687
>     #67 0x7f40c7732400 in __libc_start_main (/lib64/libc.so.6+0x20400)
>     #68 0x40d369 in _start (/home/j/w/co/emacs/src/temacs+0x40d369)
> 
> Address 0x7ffd5cc4d928 is located in stack of thread T0 at offset 136 in frame
>     #0 0x9c8c15 in child_setup /home/j/w/co/emacs/src/callproc.c:1179
> 
>   This frame has 2 object(s):
>     [32, 40) 'display'
>     [96, 104) 'tmp' <== Memory access at offset 136 overflows this variable
> HINT: this may be a false positive if your program uses some custom
> stack unwind mechanism or swapcontext

I admit that I don't understand this report.  At the point where the
report claims there was buffer overflow, child_setup is not in the
call stack, because it is/was called in another process after vfork:
the callstack shows the stack of the Emacs process, whereas
child_setup is called by a child process.  So either the report shows
a stack of a wrong process, or something else is going on.  Or maybe I
simply don't understand how to read this report.

Re: building/using address-sanitizer-enabled emacs?

[Jim Meyering]

On Tue, May 9, 2017 at 8:18 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Jim Meyering <jim@meyering.net>
>> Date: Mon, 8 May 2017 22:48:06 -0700
>> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel <emacs-devel@gnu.org>
>>
>>   echo foo |gpg -c > foo.gpg
>>   src/temacs -q foo.gpg 2> err
>>
>> which reported this stack buffer overrun:
>>
>> ==24522==ERROR: AddressSanitizer: stack-buffer-overflow on address
>> 0x7ffd5cc4d928 at pc 0x00000073fd76 bp 0x7ffd5cc4d910 sp
>> 0x7ffd5cc4d908
>> READ of size 8 at 0x7ffd5cc4d928 thread T0
>>     #0 0x73fd75 in PSEUDOVECTORP /home/j/w/co/emacs/src/lisp.h:1454
>>     #1 0x7438c9 in BUFFERP /home/j/w/co/emacs/src/buffer.h:887
>>     #2 0x9c64b6 in call_process /home/j/w/co/emacs/src/callproc.c:702
>>     #3 0x9c41db in Fcall_process /home/j/w/co/emacs/src/callproc.c:270
>>     #4 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
>>     #5 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>>     #6 0x8e072d in Fapply /home/j/w/co/emacs/src/eval.c:2375
>>     #7 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
>>     #8 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>>     #9 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #10 0x8e4b55 in funcall_lambda /home/j/w/co/emacs/src/eval.c:3022
>>     #11 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #12 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #13 0x8e4b55 in funcall_lambda /home/j/w/co/emacs/src/eval.c:3022
>>     #14 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #15 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #16 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #17 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #18 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #19 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #20 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #21 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #22 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #23 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #24 0x8e072d in Fapply /home/j/w/co/emacs/src/eval.c:2375
>>     #25 0x8e3122 in funcall_subr /home/j/w/co/emacs/src/eval.c:2799
>>     #26 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>>     #27 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #28 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #29 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #30 0x8e1f8d in call6 /home/j/w/co/emacs/src/eval.c:2649
>>     #31 0x8034eb in Finsert_file_contents /home/j/w/co/emacs/src/fileio.c:3602
>>     #32 0x8e367f in funcall_subr /home/j/w/co/emacs/src/eval.c:2831
>>     #33 0x8e2aa1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2744
>>     #34 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #35 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #36 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #37 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #38 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #39 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #40 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #41 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #42 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #43 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #44 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #45 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #46 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #47 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #48 0x8e2ae1 in Ffuncall /home/j/w/co/emacs/src/eval.c:2746
>>     #49 0x98c89b in exec_byte_code /home/j/w/co/emacs/src/bytecode.c:641
>>     #50 0x8e4604 in funcall_lambda /home/j/w/co/emacs/src/eval.c:2944
>>     #51 0x8e3f96 in apply_lambda /home/j/w/co/emacs/src/eval.c:2881
>>     #52 0x8df8ab in eval_sub /home/j/w/co/emacs/src/eval.c:2265
>>     #53 0x8dda2c in Feval /home/j/w/co/emacs/src/eval.c:2042
>>     #54 0x8df175 in eval_sub /home/j/w/co/emacs/src/eval.c:2223
>>     #55 0x945574 in readevalloop /home/j/w/co/emacs/src/lread.c:1947
>>     #56 0x9425f5 in Fload /home/j/w/co/emacs/src/lread.c:1352
>>     #57 0x8df41e in eval_sub /home/j/w/co/emacs/src/eval.c:2234
>>     #58 0x8dda2c in Feval /home/j/w/co/emacs/src/eval.c:2042
>>     #59 0x751a34 in top_level_2 /home/j/w/co/emacs/src/keyboard.c:1121
>>     #60 0x8d9c05 in internal_condition_case /home/j/w/co/emacs/src/eval.c:1326
>>     #61 0x751a97 in top_level_1 /home/j/w/co/emacs/src/keyboard.c:1129
>>     #62 0x8d83e9 in internal_catch /home/j/w/co/emacs/src/eval.c:1091
>>     #63 0x751899 in command_loop /home/j/w/co/emacs/src/keyboard.c:1090
>>     #64 0x75033f in recursive_edit_1 /home/j/w/co/emacs/src/keyboard.c:697
>>     #65 0x7506dd in Frecursive_edit /home/j/w/co/emacs/src/keyboard.c:768
>>     #66 0x74bbb9 in main /home/j/w/co/emacs/src/emacs.c:1687
>>     #67 0x7f40c7732400 in __libc_start_main (/lib64/libc.so.6+0x20400)
>>     #68 0x40d369 in _start (/home/j/w/co/emacs/src/temacs+0x40d369)
>>
>> Address 0x7ffd5cc4d928 is located in stack of thread T0 at offset 136 in frame
>>     #0 0x9c8c15 in child_setup /home/j/w/co/emacs/src/callproc.c:1179
>>
>>   This frame has 2 object(s):
>>     [32, 40) 'display'
>>     [96, 104) 'tmp' <== Memory access at offset 136 overflows this variable
>> HINT: this may be a false positive if your program uses some custom
>> stack unwind mechanism or swapcontext
>
> I admit that I don't understand this report.  At the point where the
> report claims there was buffer overflow, child_setup is not in the
> call stack, because it is/was called in another process after vfork:
> the callstack shows the stack of the Emacs process, whereas
> child_setup is called by a child process.  So either the report shows
> a stack of a wrong process, or something else is going on.  Or maybe I
> simply don't understand how to read this report.

Thanks for looking.
I confess I have not done so. Does this caveat apply?

  HINT: this may be a false positive if your program uses some custom
  stack unwind mechanism or swapcontext

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> From: Jim Meyering <jim@meyering.net>
> Date: Tue, 9 May 2017 10:06:29 -0700
> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel <emacs-devel@gnu.org>
> 
> > I admit that I don't understand this report.  At the point where the
> > report claims there was buffer overflow, child_setup is not in the
> > call stack, because it is/was called in another process after vfork:
> > the callstack shows the stack of the Emacs process, whereas
> > child_setup is called by a child process.  So either the report shows
> > a stack of a wrong process, or something else is going on.  Or maybe I
> > simply don't understand how to read this report.
> 
> Thanks for looking.
> I confess I have not done so. Does this caveat apply?
> 
>   HINT: this may be a false positive if your program uses some custom
>   stack unwind mechanism or swapcontext

I don't think I understand this caveat, either.  Does "swapcontext"
include what happens after vfork?  IOW, is this tool known to follow
forks and track each of the 2 processes separately?

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

On 05/08/2017 10:48 PM, Jim Meyering wrote:
> and tried to use it on a foo.gpg file like this:
>
>    echo foo |gpg -c > foo.gpg
>    src/temacs -q foo.gpg 2> err

My own impression from a while ago is that the address sanitizer was not 
suitable for the class Emacs undumping, since its shadow memory won't 
survive dump-restore correctly. It sort of worked in some cases but it 
didn't work in general. I suppose it may be possible to work around the 
various issues, but as I recall it'd be a lot of trouble.

Re: building/using address-sanitizer-enabled emacs?

[Jim Meyering]

On Tue, May 9, 2017 at 12:22 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 05/08/2017 10:48 PM, Jim Meyering wrote:
>>
>> and tried to use it on a foo.gpg file like this:
>>
>>    echo foo |gpg -c > foo.gpg
>>    src/temacs -q foo.gpg 2> err
>
> My own impression from a while ago is that the address sanitizer was not
> suitable for the class Emacs undumping, since its shadow memory won't
> survive dump-restore correctly. It sort of worked in some cases but it
> didn't work in general. I suppose it may be possible to work around the
> various issues, but as I recall it'd be a lot of trouble.

With the above, I thought I was avoiding the undumping issue by using temacs.

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> From: Jim Meyering <jim@meyering.net>
> Date: Tue, 9 May 2017 15:49:34 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel <emacs-devel@gnu.org>
> 
> > My own impression from a while ago is that the address sanitizer was not
> > suitable for the class Emacs undumping, since its shadow memory won't
> > survive dump-restore correctly. It sort of worked in some cases but it
> > didn't work in general. I suppose it may be possible to work around the
> > various issues, but as I recall it'd be a lot of trouble.
> 
> With the above, I thought I was avoiding the undumping issue by using temacs.

Indeed, I believe the problem you reported is unrelated to unexec.

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

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

On 05/09/2017 07:41 PM, Eli Zaretskii wrote:
>> From: Jim Meyering<jim@meyering.net>
>> With the above, I thought I was avoiding the undumping issue by using temacs.
> Indeed, I believe the problem you reported is unrelated to unexec.

You're right, I didn't read Jim's message carefully enough. I see that 
the AddressSanitizer code suffered some bitrot since I last got it 
working; among other things its output was being discarded on Fedora 25 
x86-64, which at first gave me a false sense of security.... I installed 
the attached patches to work around the GCC problem that Jim reported, 
the improper output discard, and one minor memory leak uncovered by 
AddressSanitizer.  When I run Emacs with AddressSanitizer now, it 
reports some other minor memory leaks, almost all from the fontconfig 
library. I do not observe any problems with SAFE_ALLOCA.


[-- Attachment #2: 0001-Simplify-procname-code-to-avoid-GCC-bug.patch --]
[-- Type: text/x-patch, Size: 3918 bytes --]

From be9e60fc3c43cc49cc5d749924c3e96737ae297c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 16 May 2017 14:29:18 -0700
Subject: [PATCH 1/3] Simplify procname code to avoid GCC bug

* src/process.c (server_accept_connection): Simplify and avoid
multiple calls and struct literals in the last case of a switch.
The old code ran afoul of GCC bug 80659, which caused an internal
compiler error.  Problem reported by Jim Meyering in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00182.html
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80659
---
 src/process.c | 53 ++++++++++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/src/process.c b/src/process.c
index 4a28639..fdea977 100644
--- a/src/process.c
+++ b/src/process.c
@@ -4659,7 +4659,7 @@ static EMACS_INT connect_counter = 0;
 static void
 server_accept_connection (Lisp_Object server, int channel)
 {
-  Lisp_Object proc, caller, name, buffer;
+  Lisp_Object buffer;
   Lisp_Object contact, host, service;
   struct Lisp_Process *ps = XPROCESS (server);
   struct Lisp_Process *p;
@@ -4701,49 +4701,43 @@ server_accept_connection (Lisp_Object server, int channel)
      information for this process.  */
   host = Qt;
   service = Qnil;
+  Lisp_Object args[11];
+  int nargs = 0;
+  AUTO_STRING (procname_format_in, "%s <%d.%d.%d.%d:%d>");
+  AUTO_STRING (procname_format_in6, "%s <[%x:%x:%x:%x:%x:%x:%x:%x]:%d>");
+  AUTO_STRING (procname_format_default, "%s <%d>");
   switch (saddr.sa.sa_family)
     {
     case AF_INET:
       {
+	args[nargs++] = procname_format_in;
+	nargs++;
 	unsigned char *ip = (unsigned char *)&saddr.in.sin_addr.s_addr;
-
-	AUTO_STRING (ipv4_format, "%d.%d.%d.%d");
-	host = CALLN (Fformat, ipv4_format,
-		      make_number (ip[0]), make_number (ip[1]),
-		      make_number (ip[2]), make_number (ip[3]));
 	service = make_number (ntohs (saddr.in.sin_port));
-	AUTO_STRING (caller_format, " <%s:%d>");
-	caller = CALLN (Fformat, caller_format, host, service);
+	for (int i = 0; i < 4; i++)
+	  args[nargs++] = make_number (ip[i]);
+	args[nargs++] = service;
       }
       break;
 
 #ifdef AF_INET6
     case AF_INET6:
       {
-	Lisp_Object args[9];
+	args[nargs++] = procname_format_in6;
+	nargs++;
 	uint16_t *ip6 = (uint16_t *)&saddr.in6.sin6_addr;
-	int i;
-
-	AUTO_STRING (ipv6_format, "%x:%x:%x:%x:%x:%x:%x:%x");
-	args[0] = ipv6_format;
-	for (i = 0; i < 8; i++)
-	  args[i + 1] = make_number (ntohs (ip6[i]));
-	host = CALLMANY (Fformat, args);
 	service = make_number (ntohs (saddr.in.sin_port));
-	AUTO_STRING (caller_format, " <[%s]:%d>");
-	caller = CALLN (Fformat, caller_format, host, service);
+	for (int i = 0; i < 8; i++)
+	  args[nargs++] = make_number (ip6[i]);
+	args[nargs++] = service;
       }
       break;
 #endif
 
-#ifdef HAVE_LOCAL_SOCKETS
-    case AF_LOCAL:
-#endif
     default:
-      caller = Fnumber_to_string (make_number (connect_counter));
-      AUTO_STRING (space_less_than, " <");
-      AUTO_STRING (greater_than, ">");
-      caller = concat3 (space_less_than, caller, greater_than);
+      args[nargs++] = procname_format_default;
+      nargs++;
+      args[nargs++] = make_number (connect_counter);
       break;
     }
 
@@ -4764,16 +4758,17 @@ server_accept_connection (Lisp_Object server, int channel)
 	buffer = ps->name;
       if (!NILP (buffer))
 	{
-	  buffer = concat2 (buffer, caller);
-	  buffer = Fget_buffer_create (buffer);
+	  args[1] = buffer;
+	  buffer = Fget_buffer_create (Fformat (nargs, args));
 	}
     }
 
   /* Generate a unique name for the new server process.  Combine the
      server process name with the caller identification.  */
 
-  name = concat2 (ps->name, caller);
-  proc = make_process (name);
+  args[1] = ps->name;
+  Lisp_Object name = Fformat (nargs, args);
+  Lisp_Object proc = make_process (name);
 
   chan_process[s] = proc;
 
-- 
2.9.4


[-- Attachment #3: 0002-Do-not-discard-AddressSanitizer-stderr.patch --]
[-- Type: text/x-patch, Size: 887 bytes --]

From 69d0a8500cabc4c034e2d6d873af54a8e8362e3b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 16 May 2017 14:30:37 -0700
Subject: [PATCH 2/3] Do not discard AddressSanitizer stderr

* src/emacs.c (close_output_streams) [ADDRESS_SANITIZER]:
Do not close stderr.
---
 src/emacs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/emacs.c b/src/emacs.c
index 9339d60..3aa914f 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -657,8 +657,11 @@ close_output_streams (void)
       _exit (EXIT_FAILURE);
     }
 
-   if (close_stream (stderr) != 0)
-     _exit (EXIT_FAILURE);
+  /* Do not close stderr if addresses are being sanitized, as the
+     sanitizer might report to stderr after this function is
+     invoked.  */
+  if (!ADDRESS_SANITIZER && close_stream (stderr) != 0)
+    _exit (EXIT_FAILURE);
 }
 
 /* ARGSUSED */
-- 
2.9.4


[-- Attachment #4: 0003-Fix-minor-timezone-memory-leak.patch --]
[-- Type: text/x-patch, Size: 1216 bytes --]

From f7c07930b581b1bcfdfb1874b6883233516bdf11 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 16 May 2017 14:19:36 -0700
Subject: [PATCH 3/3] Fix minor timezone memory leak

* src/editfns.c (wall_clock_tz): Remove; unused.
---
 src/editfns.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/editfns.c b/src/editfns.c
index ecb8e3f..75eb75a 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -81,10 +81,8 @@ static Lisp_Object styled_format (ptrdiff_t, Lisp_Object *, bool);
 
 enum { tzeqlen = sizeof "TZ=" - 1 };
 
-/* Time zones equivalent to current local time, to wall clock time,
-   and to UTC, respectively.  */
+/* Time zones equivalent to current local time and to UTC, respectively.  */
 static timezone_t local_tz;
-static timezone_t wall_clock_tz;
 static timezone_t const utc_tz = 0;
 
 /* The cached value of Vsystem_name.  This is used only to compare it
@@ -269,7 +267,6 @@ init_editfns (bool dumping)
 
   /* Set the time zone rule now, so that the call to putenv is done
      before multiple threads are active.  */
-  wall_clock_tz = xtzalloc (0);
   tzlookup (tz ? build_string (tz) : Qwall, true);
 
   pw = getpwuid (getuid ());
-- 
2.9.4

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 16 May 2017 14:49:46 -0700
> 
> > Indeed, I believe the problem you reported is unrelated to unexec.
> 
> You're right, I didn't read Jim's message carefully enough. I see that 
> the AddressSanitizer code suffered some bitrot since I last got it 
> working; among other things its output was being discarded on Fedora 25 
> x86-64, which at first gave me a false sense of security.... I installed 
> the attached patches to work around the GCC problem that Jim reported, 
> the improper output discard, and one minor memory leak uncovered by 
> AddressSanitizer.  When I run Emacs with AddressSanitizer now, it 
> reports some other minor memory leaks, almost all from the fontconfig 
> library. I do not observe any problems with SAFE_ALLOCA.

Thanks.  But isn't it strange that none of the changes you made is
related to starting a sub-process, which was where the original report
came from?

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

On 05/16/2017 07:24 PM, Eli Zaretskii wrote:
> isn't it strange that none of the changes you made is
> related to starting a sub-process, which was where the original report
> came from?

Although Jim's original report mentioned process.c, the referenced code 
was in server_accept_connection (not starting a sub-process), and that 
code was indeed affected by one of the recent changes I made (commit 
be9e60fc3c43cc49cc5d749924c3e96737ae297c).

Jim's original report mainly focused on a buffer overflow in dumping. 
That part has never worked with AddressSanitizer; one must configure 
with CANNOT_DUMP=yes to use AddressSanitizer.

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> Cc: jim@meyering.net, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 17 May 2017 07:46:49 -0700
> 
> On 05/16/2017 07:24 PM, Eli Zaretskii wrote:
> > isn't it strange that none of the changes you made is
> > related to starting a sub-process, which was where the original report
> > came from?
> 
> Although Jim's original report mentioned process.c, the referenced code 
> was in server_accept_connection (not starting a sub-process), and that 
> code was indeed affected by one of the recent changes I made (commit 
> be9e60fc3c43cc49cc5d749924c3e96737ae297c).

I think we might be talking about 2 different reports.  I meant the
one here:

  http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00246.html

The reproducing recipe given there by Jim was this:

  echo foo |gpg -c > foo.gpg
  src/temacs -q foo.gpg 2> err

and I guessed that visiting foo.gpg starts gpg as a subprocess.  The
backtrace posted by Jim, viz.:

  ==24522==ERROR: AddressSanitizer: stack-buffer-overflow on address
  0x7ffd5cc4d928 at pc 0x00000073fd76 bp 0x7ffd5cc4d910 sp
  0x7ffd5cc4d908
  READ of size 8 at 0x7ffd5cc4d928 thread T0
      #0 0x73fd75 in PSEUDOVECTORP /home/j/w/co/emacs/src/lisp.h:1454
      #1 0x7438c9 in BUFFERP /home/j/w/co/emacs/src/buffer.h:887
      #2 0x9c64b6 in call_process /home/j/w/co/emacs/src/callproc.c:702
      #3 0x9c41db in Fcall_process /home/j/w/co/emacs/src/callproc.c:270

seems to confirm my guess.  Did I miss something?

Re: building/using address-sanitizer-enabled emacs?

[Paul Eggert]

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

On 05/17/2017 09:06 AM, Eli Zaretskii wrote:
> I think we might be talking about 2 different reports. I meant the
> one here:
>
>    http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00246.html

Ah, that came after his original report 
<http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00182.html>, 
which is the one I was referring to. I reproduced the later problem, and 
as near as I can make it out it is due to an incompatibility between 
vfork and -fsanitize=address. My guess is that the vforked child 
corrupts the parent's shadow memory. I worked around the problem by 
installing the attached patch into Emacs.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Work-around-AddressSanitizer-bug-with-vfork.patch --]
[-- Type: text/x-patch; name="0001-Work-around-AddressSanitizer-bug-with-vfork.patch", Size: 1353 bytes --]

From 709259dcc501ef991991a35a6ffb2aef02a62c60 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 17 May 2017 10:58:11 -0700
Subject: [PATCH] Work around AddressSanitizer bug with vfork
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Jim Meyering in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00246.html
* src/conf_post.h (vfork) [ADDRESS_SANITIZER]: Define to fork.
Unfortunately with the AddressSanitizer in Fedora 25 x86-64, the
vforked child messes up the parent’s shadow memory.  This is too
bad, as we’d rather have AddressSanitizer catch memory-access bugs
related to vfork.
---
 src/conf_post.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/conf_post.h b/src/conf_post.h
index 4fc0428..1462bd1 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -302,6 +302,12 @@ extern int emacs_setenv_TZ (char const *);
 # define ATTRIBUTE_NO_SANITIZE_ADDRESS
 #endif
 
+/* gcc -fsanitize=address does not work with vfork in Fedora 25 x86-64.
+   For now, assume that this problem occurs on all platforms.  */
+#if ADDRESS_SANITIZER && !defined vfork
+# define vfork fork
+#endif
+
 /* Some versions of GNU/Linux define noinline in their headers.  */
 #ifdef noinline
 #undef noinline
-- 
2.9.4

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> Cc: jim@meyering.net, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 17 May 2017 13:05:29 -0700
> 
> On 05/17/2017 09:06 AM, Eli Zaretskii wrote:
> > I think we might be talking about 2 different reports. I meant the
> > one here:
> >
> >    http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00246.html
> 
> Ah, that came after his original report 
> <http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00182.html>, 
> which is the one I was referring to. I reproduced the later problem, and 
> as near as I can make it out it is due to an incompatibility between 
> vfork and -fsanitize=address. My guess is that the vforked child 
> corrupts the parent's shadow memory. I worked around the problem by 
> installing the attached patch into Emacs.

Yes, that makes sense, given the details in the reports, which indeed
hinted on such an incompatibility.

Thanks.

Re: building/using address-sanitizer-enabled emacs?

[Philipp Stephani]

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

Jim Meyering <jim@meyering.net> schrieb am So., 7. Mai 2017 um 05:40 Uhr:

> Has anyone managed to dump an ASAN-enabled emacs recently?
> I can build and use an ASAN-enabled temacs, but it's too slow, of course.


I've just run the test suite through ASan with an undumped emacs on macOS.
I got lots of dynamic-stack-buffer-overflow errors when using SAFE_ALLOCA,
which I can't explain; maybe that's indeed an ASan bug because alloca isn't
that widely used.
There's one actual bug: the sockaddr_in in line 3538 of process.c needs to
be sockaddr_storage, otherwise it's too small for IPv6. (And
conv_sockaddr_to_lisp should probably check that the address is of the
expected length and not blindly overwrite the len argument.)

[-- Attachment #2: Type: text/html, Size: 1032 bytes --]

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 09 May 2017 23:15:26 +0000
> 
> There's one actual bug: the sockaddr_in in line 3538 of process.c needs to be sockaddr_storage, otherwise
> it's too small for IPv6. (And conv_sockaddr_to_lisp should probably check that the address is of the expected
> length and not blindly overwrite the len argument.)

Please show the detailed analysis, as I looked into that once and
concluded that the code is correct.

Re: building/using address-sanitizer-enabled emacs?

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 10. Mai 2017 um 04:43 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Tue, 09 May 2017 23:15:26 +0000
> >
> > There's one actual bug: the sockaddr_in in line 3538 of process.c needs
> to be sockaddr_storage, otherwise
> > it's too small for IPv6. (And conv_sockaddr_to_lisp should probably
> check that the address is of the expected
> > length and not blindly overwrite the len argument.)
>
> Please show the detailed analysis, as I looked into that once and
> concluded that the code is correct.
>

The full report is

=================================================================
==31024==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7fff5fbfa690 at pc 0x0001003e6baf bp 0x7fff5fbfa4f0 sp 0x7fff5fbfa4e8
READ of size 2 at 0x7fff5fbfa690 thread T0
    #0 0x1003e6bae in conv_sockaddr_to_lisp src/process.c:2497:34
    #1 0x1003eb9f8 in connect_network_socket src/process.c:3542:7
    #2 0x1003e9864 in Fmake_network_process src/process.c:4154:5
    #3 0x10035e417 in eval_sub src/eval.c:2191:10
    #4 0x10035f1a7 in Fsetq src/eval.c:511:10
    #5 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #6 0x10035efbf in Fprogn src/eval.c:449:13
    #7 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #8 0x100361df4 in internal_lisp_condition_case src/eval.c:1297:24
    #9 0x100361a85 in Fcondition_case src/eval.c:1221:10
    #10 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #11 0x10035e02f in eval_sub src/eval.c:2208:21
    #12 0x10035ef16 in Fand src/eval.c:384:13
    #13 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #14 0x100360e44 in Fwhile src/eval.c:979:17
    #15 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #16 0x10035efbf in Fprogn src/eval.c:449:13
    #17 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #18 0x100361a28 in Funwind_protect src/eval.c:1185:9
    #19 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #20 0x10035efbf in Fprogn src/eval.c:449:13
    #21 0x100360d2c in Flet src/eval.c:963:9
    #22 0x10035dfd1 in eval_sub src/eval.c:2173:8
    #23 0x10035efbf in Fprogn src/eval.c:449:13
    #24 0x10036766f in funcall_lambda src/eval.c:3015:11
    #25 0x100364efb in Ffuncall src/eval.c:2746:11
    #26 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #27 0x1003673de in funcall_lambda src/eval.c:2944:11
    #28 0x100364efb in Ffuncall src/eval.c:2746:11
    #29 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #30 0x1003673de in funcall_lambda src/eval.c:2944:11
    #31 0x100364efb in Ffuncall src/eval.c:2746:11
    #32 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #33 0x1003673de in funcall_lambda src/eval.c:2944:11
    #34 0x100364efb in Ffuncall src/eval.c:2746:11
    #35 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #36 0x1003673de in funcall_lambda src/eval.c:2944:11
    #37 0x100364efb in Ffuncall src/eval.c:2746:11
    #38 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #39 0x1003673de in funcall_lambda src/eval.c:2944:11
    #40 0x100364efb in Ffuncall src/eval.c:2746:11
    #41 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #42 0x1003673de in funcall_lambda src/eval.c:2944:11
    #43 0x1003643df in apply_lambda src/eval.c:2881:9
    #44 0x10035df97 in eval_sub src/eval.c:2265:12
    #45 0x100363d85 in Feval src/eval.c:2042:28
    #46 0x100366af0 in funcall_subr src/eval.c:2821:19
    #47 0x100364f15 in Ffuncall src/eval.c:2744:11
    #48 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #49 0x1003673de in funcall_lambda src/eval.c:2944:11
    #50 0x100364efb in Ffuncall src/eval.c:2746:11
    #51 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #52 0x1003673de in funcall_lambda src/eval.c:2944:11
    #53 0x100364efb in Ffuncall src/eval.c:2746:11
    #54 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
    #55 0x1003673de in funcall_lambda src/eval.c:2944:11
    #56 0x1003643df in apply_lambda src/eval.c:2881:9
    #57 0x10035df97 in eval_sub src/eval.c:2265:12
    #58 0x100363d85 in Feval src/eval.c:2042:28
    #59 0x10035e680 in eval_sub src/eval.c:2223:15
    #60 0x1003a6944 in readevalloop src/lread.c:1947:15
    #61 0x1003a410c in Fload src/lread.c:1359:7
    #62 0x10035e917 in eval_sub src/eval.c:2234:15
    #63 0x100363d85 in Feval src/eval.c:2042:28
    #64 0x10026f0b2 in top_level_2 src/keyboard.c:1121:10
    #65 0x1003621f4 in internal_condition_case src/eval.c:1326:25
    #66 0x10026eaa4 in top_level_1 src/keyboard.c:1129:5
    #67 0x100361411 in internal_catch src/eval.c:1091:25
    #68 0x100249135 in command_loop src/keyboard.c:1090:2
    #69 0x100248f21 in recursive_edit_1 src/keyboard.c:697:9
    #70 0x100249480 in Frecursive_edit src/keyboard.c:768:3
    #71 0x1002453a1 in main src/emacs.c:1687:3
    #72 0x7fffae305234 in start
(/usr/lib/system/libdyld.dylib:x86_64+0x5234)

Address 0x7fff5fbfa690 is located in stack of thread T0 at offset 336 in
frame
    #0 0x1003eabbf in connect_network_socket src/process.c:3307

  This frame has 10 object(s):
    [32, 36) 'xerrno'
    [48, 52) 'family'
    [64, 68) 'optval'
    [80, 96) 'sa1'
    [112, 116) 'len1'
    [128, 132) 'len'
    [144, 272) 'fdset'
    [304, 308) 'rfamily'
    [320, 336) 'sa1261' <== Memory access at offset 336 overflows this
variable
    [352, 356) 'len1262'
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow src/process.c:2497:34 in
conv_sockaddr_to_lisp
Shadow bytes around the buggy address:
  0x1fffebf7f480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffebf7f490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffebf7f4a0: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 04 f2
  0x1fffebf7f4b0: 04 f2 00 00 f2 f2 04 f2 04 f2 00 00 00 00 00 00
  0x1fffebf7f4c0: 00 00 00 00 00 00 00 00 00 00 f2 f2 f2 f2 04 f2
=>0x1fffebf7f4d0: 00 00[f2]f2 04 f3 f3 f3 00 00 00 00 00 00 00 00
  0x1fffebf7f4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffebf7f4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffebf7f500: 00 00 00 00 f1 f1 f1 f1 00 00 05 f2 f2 f2 f2 f2
  0x1fffebf7f510: 04 f2 00 f2 f2 f2 00 00 00 00 00 00 f2 f2 f2 f2
  0x1fffebf7f520: 00 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00 f3 f3
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==31024==ABORTING

The problem is here:

 struct sockaddr_in sa1;
 socklen_t len1 = sizeof (sa1);
 if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
   contact = Fplist_put (contact, QClocal,
 conv_sockaddr_to_lisp ((struct sockaddr *)&sa1, len1));

sockaddr_in is too small for IPv6 addresses, so getsockname doesn't fill it
out completely. But conv_sockaddr_to_lisp only looks at the address family
and attempts to read out the entire IPv6 address, reading past the sa1
variable memory. Thus this needs to be sockaddr_storage, which is
guaranteed to be large enough for all supported addresses.
Probably there should also be an eassert(len1 <= sizeof sa1) after the call
to getsockname, just to make sure.

[-- Attachment #2: Type: text/html, Size: 10092 bytes --]

Re: building/using address-sanitizer-enabled emacs?

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed, 10 May 2017 22:24:49 +0000
> Cc: jim@meyering.net, emacs-devel@gnu.org
> 
>  Please show the detailed analysis, as I looked into that once and
>  concluded that the code is correct.
> 
> The full report is
> 
> =================================================================
> ==31024==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff5fbfa690 at pc
> 0x0001003e6baf bp 0x7fff5fbfa4f0 sp 0x7fff5fbfa4e8
> READ of size 2 at 0x7fff5fbfa690 thread T0
> #0 0x1003e6bae in conv_sockaddr_to_lisp src/process.c:2497:34
> [...]
> The problem is here:
> 
> struct sockaddr_in sa1;
> socklen_t len1 = sizeof (sa1);
> if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
> contact = Fplist_put (contact, QClocal,
> conv_sockaddr_to_lisp ((struct sockaddr *)&sa1, len1));
> 
> sockaddr_in is too small for IPv6 addresses, so getsockname doesn't fill it out completely. But
> conv_sockaddr_to_lisp only looks at the address family and attempts to read out the entire IPv6 address,
> reading past the sa1 variable memory. Thus this needs to be sockaddr_storage, which is guaranteed to be
> large enough for all supported addresses.
> Probably there should also be an eassert(len1 <= sizeof sa1) after the call to getsockname, just to make
> sure. 

Indeed, I believe you are right.

[PATCH] Fix use of sockaddr_in

[Philipp Stephani]

Fixes an access violation detected by AddressSanitizer.

* process.c (connect_network_socket): Use sockaddr_storage
structure instead of sockaddr_in.  Only sockaddr_storage is
guaranteed to be large enough for all address families.
---
 src/process.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/process.c b/src/process.c
index 0edd092ef6..873db48b55 100644
--- a/src/process.c
+++ b/src/process.c
@@ -3420,16 +3420,35 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
 #ifdef HAVE_GETSOCKNAME
 	  if (p->port == 0)
 	    {
-	      struct sockaddr_in sa1;
+	      struct sockaddr_storage sa1;
 	      socklen_t len1 = sizeof (sa1);
 	      if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
 		{
-		  Lisp_Object service;
-		  service = make_number (ntohs (sa1.sin_port));
-		  contact = Fplist_put (contact, QCservice, service);
-		  /* Save the port number so that we can stash it in
-		     the process object later.  */
-		  ((struct sockaddr_in *)sa)->sin_port = sa1.sin_port;
+                  eassert (sizeof sa1 >= len1);
+                  in_port_t port;
+                  bool has_port;
+                  switch (sa1.ss_family)
+                    {
+                    case AF_INET:
+                      port = ((struct sockaddr_in6 *) (struct sockaddr *) &sa1)->sin6_port;
+                      has_port = true;
+                      break;
+                    case AF_INET6:
+                      port = ((struct sockaddr_in *) (struct sockaddr *) &sa1)->sin_port;
+                      has_port = true;
+                      break;
+                    default:
+                      has_port = false;
+                      break;
+                    }
+                  if (has_port)
+                    {
+                      const Lisp_Object service = make_number (ntohs (port));
+                      contact = Fplist_put (contact, QCservice, service);
+                      /* Save the port number so that we can stash it
+                         in the process object later.  */
+                      ((struct sockaddr_in *)sa)->sin_port = port;
+                    }
 		}
 	    }
 #endif
@@ -3535,11 +3554,14 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
 #ifdef HAVE_GETSOCKNAME
       if (!p->is_server)
 	{
-	  struct sockaddr_in sa1;
+	  struct sockaddr_storage sa1;
 	  socklen_t len1 = sizeof (sa1);
 	  if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
-	    contact = Fplist_put (contact, QClocal,
-				  conv_sockaddr_to_lisp ((struct sockaddr *)&sa1, len1));
+            {
+              eassert (sizeof sa1 >= len1);
+              contact = Fplist_put (contact, QClocal,
+                                    conv_sockaddr_to_lisp ((struct sockaddr *)&sa1, len1));
+            }
 	}
 #endif
     }
-- 
2.13.0

Re: [PATCH] Fix use of sockaddr_in

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 13 May 2017 17:08:37 +0200
> Cc: Philipp Stephani <phst@google.com>
> 
> Fixes an access violation detected by AddressSanitizer.
> 
> * process.c (connect_network_socket): Use sockaddr_storage
> structure instead of sockaddr_in.  Only sockaddr_storage is
> guaranteed to be large enough for all address families.

Thanks.

The first hunk doesn't seem to be necessary, though: in both IPv4 and
IPv6 structures, the port is at the same offset and has the same size.
So I'd rather put an assertion there, to verify that the offsets and
sizes are the same (because the standards don't guarantee that), and
that's all.

Re: [PATCH] Fix use of sockaddr_in

[Andreas Schwab]

On Mai 13 2017, Eli Zaretskii <eliz@gnu.org> wrote:

> The first hunk doesn't seem to be necessary, though: in both IPv4 and
> IPv6 structures, the port is at the same offset and has the same size.

That isn't true for AF_UNIX addresses.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix use of sockaddr_in

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Philipp Stephani <p.stephani2@gmail.com>,  phst@google.com,  emacs-devel@gnu.org
> Date: Sat, 13 May 2017 21:14:33 +0200
> 
> On Mai 13 2017, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > The first hunk doesn't seem to be necessary, though: in both IPv4 and
> > IPv6 structures, the port is at the same offset and has the same size.
> 
> That isn't true for AF_UNIX addresses.

Can that happen at this point in the code?

Re: [PATCH] Fix use of sockaddr_in

[Andreas Schwab]

On Mai 13 2017, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: Philipp Stephani <p.stephani2@gmail.com>,  phst@google.com,  emacs-devel@gnu.org
>> Date: Sat, 13 May 2017 21:14:33 +0200
>> 
>> On Mai 13 2017, Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> > The first hunk doesn't seem to be necessary, though: in both IPv4 and
>> > IPv6 structures, the port is at the same offset and has the same size.
>> 
>> That isn't true for AF_UNIX addresses.
>
> Can that happen at this point in the code?

Yes.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix use of sockaddr_in

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: p.stephani2@gmail.com,  phst@google.com,  emacs-devel@gnu.org
> Date: Sat, 13 May 2017 22:05:37 +0200
> 
> On Mai 13 2017, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Andreas Schwab <schwab@linux-m68k.org>
> >> Cc: Philipp Stephani <p.stephani2@gmail.com>,  phst@google.com,  emacs-devel@gnu.org
> >> Date: Sat, 13 May 2017 21:14:33 +0200
> >> 
> >> On Mai 13 2017, Eli Zaretskii <eliz@gnu.org> wrote:
> >> 
> >> > The first hunk doesn't seem to be necessary, though: in both IPv4 and
> >> > IPv6 structures, the port is at the same offset and has the same size.
> >> 
> >> That isn't true for AF_UNIX addresses.
> >
> > Can that happen at this point in the code?
> 
> Yes.

But the original patch (and the original code) didn't handle that
case.

Re: [PATCH] Fix use of sockaddr_in

[Andreas Schwab]

On Mai 14 2017, Eli Zaretskii <eliz@gnu.org> wrote:

> But the original patch (and the original code) didn't handle that
> case.

What do you want to say?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix use of sockaddr_in

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: p.stephani2@gmail.com,  phst@google.com,  emacs-devel@gnu.org
> Date: Sun, 14 May 2017 08:11:01 +0200
> 
> On Mai 14 2017, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > But the original patch (and the original code) didn't handle that
> > case.
> 
> What do you want to say?

I guess I'm saying that I didn't understand your comment.

AFAIU, your comment says that when getsockname is called for AF_UNIX
sockets, it returns a structure that doesn't report a port, only a
file name.  The original code didn't handle this use case, so if your
intent was to say that this condition should be added to the code,
i.e. for AF_UNIX we should not try to compute the port, then I agree.

However, this is orthogonal to my suggestion, which only affected
AF_INET and AF_INET6 cases.  For those cases, the port is at the same
offset, and adding an assertion about that is IMO all that's needed.

Re: [PATCH] Fix use of sockaddr_in

[Paul Eggert]

Eli Zaretskii wrote:
> if your
> intent was to say that this condition should be added to the code,
> i.e. for AF_UNIX we should not try to compute the port, then I agree.

I installed a modified version of Philipp's patch, to do that. Thanks, Philipp, 
for tracking this down.

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d23a486ba27405acfda67a4dc387ade5e399a29b

Re: [PATCH] Fix use of sockaddr_in

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am Mo., 15. Mai 2017 um 08:15 Uhr:

> Eli Zaretskii wrote:
> > if your
> > intent was to say that this condition should be added to the code,
> > i.e. for AF_UNIX we should not try to compute the port, then I agree.
>
> I installed a modified version of Philipp's patch, to do that. Thanks,
> Philipp,
> for tracking this down.
>
>
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d23a486ba27405acfda67a4dc387ade5e399a29b


Thanks! A few questions to guard against undefined behavior:
- Maybe add verify(INT_MAX >= TYPE_MAXIMUM(in_port_t)) and
verify(TYPE_MINIMUM(in_port_t) == 0) to make sure that we can use int for
the port?
- Should there be a guard against the alias violation, e.g. by declaring sa
and sa1 with __attribute__((may_alias))? Otherwise it's UB and the compiler
might elide the switch entirely.
-- 

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

[-- Attachment #2: Type: text/html, Size: 2114 bytes --]

Re: [PATCH] Fix use of sockaddr_in

[Paul Eggert]

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

On 05/15/2017 02:04 AM, Philipp Stephani wrote:
> - Maybe add verify(INT_MAX >= TYPE_MAXIMUM(in_port_t)) and 
> verify(TYPE_MINIMUM(in_port_t) == 0) to make sure that we can use int 
> for the port?

The code has been changed so that the port number is no longer stored in 
an int so this remark is obsolete now. However, if this issue crops up 
later I don't think we need to worry about it for IPv4 and IPv6 ports, 
since they're always in the range 0..65535. Emacs already assumes that 
'int' is at least 32 bits wide, as per POSIX.

> - Should there be a guard against the alias violation, e.g. by 
> declaring sa and sa1 with __attribute__((may_alias))? Otherwise it's 
> UB and the compiler might elide the switch entirely.

Yes, that is easy enough to do and would avoid some unlikely but 
hard-to-catch bugs. I installed the attached. Most likely other parts of 
Emacs should use may_alias too; do you happen to have any tool in your 
toolbox that would find them systematically?


[-- Attachment #2: 0001-Avoid-undefined-behavior-in-struct-sockaddr.patch --]
[-- Type: text/x-patch, Size: 6297 bytes --]

From 2c70724a4d9809d34caefa0903012d8449b62a3c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 17 May 2017 13:35:52 -0700
Subject: [PATCH] Avoid undefined behavior in struct sockaddr

Problem noted by Philipp Stephani in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00391.html
* src/conf_post.h (ATTRIBUTE_MAY_ALIAS, DECLARE_POINTER_ALIAS):
New macros.
* src/process.c (conv_sockaddr_to_lisp, conv_lisp_to_sockaddr)
(connect_network_socket, network_interface_info)
(server_accept_connection): Use it when aliasing non-char objects.
---
 src/conf_post.h | 14 ++++++++++++++
 src/process.c   | 31 +++++++++++++++++--------------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/conf_post.h b/src/conf_post.h
index 1462bd1..c05c93b 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -263,6 +263,20 @@ extern int emacs_setenv_TZ (char const *);
 #define ATTRIBUTE_CONST _GL_ATTRIBUTE_CONST
 #define ATTRIBUTE_UNUSED _GL_UNUSED
 
+#if GNUC_PREREQ (3, 3, 0)
+# define ATTRIBUTE_MAY_ALIAS __attribute__ ((__may_alias__))
+#else
+# define ATTRIBUTE_MAY_ALIAS
+#endif
+
+/* Declare NAME to be a pointer to an object of type TYPE, initialized
+   to the address ADDR, which may be of a different type.  Accesses
+   via NAME may alias with other accesses with the traditional
+   behavior, even if options like gcc -fstrict-aliasing are used.  */
+
+#define DECLARE_POINTER_ALIAS(name, type, addr) \
+  type ATTRIBUTE_MAY_ALIAS *name = (type *) (addr)
+
 #if 3 <= __GNUC__
 # define ATTRIBUTE_MALLOC __attribute__ ((__malloc__))
 #else
diff --git a/src/process.c b/src/process.c
index 8180fea..c301739 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2476,7 +2476,7 @@ conv_sockaddr_to_lisp (struct sockaddr *sa, ptrdiff_t len)
     {
     case AF_INET:
       {
-	struct sockaddr_in *sin = (struct sockaddr_in *) sa;
+	DECLARE_POINTER_ALIAS (sin, struct sockaddr_in, sa);
 	len = sizeof (sin->sin_addr) + 1;
 	address = Fmake_vector (make_number (len), Qnil);
 	p = XVECTOR (address);
@@ -2487,8 +2487,8 @@ conv_sockaddr_to_lisp (struct sockaddr *sa, ptrdiff_t len)
 #ifdef AF_INET6
     case AF_INET6:
       {
-	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) sa;
-	uint16_t *ip6 = (uint16_t *) &sin6->sin6_addr;
+	DECLARE_POINTER_ALIAS (sin6, struct sockaddr_in6, sa);
+	DECLARE_POINTER_ALIAS (ip6, uint16_t, &sin6->sin6_addr);
 	len = sizeof (sin6->sin6_addr) / 2 + 1;
 	address = Fmake_vector (make_number (len), Qnil);
 	p = XVECTOR (address);
@@ -2501,7 +2501,7 @@ conv_sockaddr_to_lisp (struct sockaddr *sa, ptrdiff_t len)
 #ifdef HAVE_LOCAL_SOCKETS
     case AF_LOCAL:
       {
-	struct sockaddr_un *sockun = (struct sockaddr_un *) sa;
+	DECLARE_POINTER_ALIAS (sockun, struct sockaddr_un, sa);
         ptrdiff_t name_length = len - offsetof (struct sockaddr_un, sun_path);
         /* If the first byte is NUL, the name is a Linux abstract
            socket name, and the name can contain embedded NULs.  If
@@ -2612,7 +2612,7 @@ conv_lisp_to_sockaddr (int family, Lisp_Object address, struct sockaddr *sa, int
       p = XVECTOR (address);
       if (family == AF_INET)
 	{
-	  struct sockaddr_in *sin = (struct sockaddr_in *) sa;
+	  DECLARE_POINTER_ALIAS (sin, struct sockaddr_in, sa);
 	  len = sizeof (sin->sin_addr) + 1;
 	  hostport = XINT (p->contents[--len]);
 	  sin->sin_port = htons (hostport);
@@ -2622,8 +2622,8 @@ conv_lisp_to_sockaddr (int family, Lisp_Object address, struct sockaddr *sa, int
 #ifdef AF_INET6
       else if (family == AF_INET6)
 	{
-	  struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) sa;
-	  uint16_t *ip6 = (uint16_t *)&sin6->sin6_addr;
+	  DECLARE_POINTER_ALIAS (sin6, struct sockaddr_in6, sa);
+	  DECLARE_POINTER_ALIAS (ip6, uint16_t, &sin6->sin6_addr);
 	  len = sizeof (sin6->sin6_addr) / 2 + 1;
 	  hostport = XINT (p->contents[--len]);
 	  sin6->sin6_port = htons (hostport);
@@ -2645,7 +2645,7 @@ conv_lisp_to_sockaddr (int family, Lisp_Object address, struct sockaddr *sa, int
 #ifdef HAVE_LOCAL_SOCKETS
       if (family == AF_LOCAL)
 	{
-	  struct sockaddr_un *sockun = (struct sockaddr_un *) sa;
+	  DECLARE_POINTER_ALIAS (sockun, struct sockaddr_un, sa);
 	  cp = SDATA (address);
 	  for (i = 0; i < sizeof (sockun->sun_path) && *cp; i++)
 	    sockun->sun_path[i] = *cp++;
@@ -3436,13 +3436,15 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
 		       == offsetof (struct sockaddr_in6, sin6_port))
 		      && sizeof (sa1.sin_port) == sizeof (sa6.sin6_port));
 #endif
-	      if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
+	      DECLARE_POINTER_ALIAS (psa1, struct sockaddr, &sa1);
+	      if (getsockname (s, psa1, &len1) == 0)
 		{
 		  Lisp_Object service = make_number (ntohs (sa1.sin_port));
 		  contact = Fplist_put (contact, QCservice, service);
 		  /* Save the port number so that we can stash it in
 		     the process object later.  */
-		  ((struct sockaddr_in *) sa)->sin_port = sa1.sin_port;
+		  DECLARE_POINTER_ALIAS (psa, struct sockaddr_in, sa);
+		  psa->sin_port = sa1.sin_port;
 		}
 	    }
 #endif
@@ -3550,9 +3552,10 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
 	{
 	  struct sockaddr_storage sa1;
 	  socklen_t len1 = sizeof (sa1);
-	  if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
+	  DECLARE_POINTER_ALIAS (psa1, struct sockaddr, &sa1);
+	  if (getsockname (s, psa1, &len1) == 0)
 	    contact = Fplist_put (contact, QClocal,
-				  conv_sockaddr_to_lisp ((struct sockaddr *)&sa1, len1));
+				  conv_sockaddr_to_lisp (psa1, len1));
 	}
 #endif
     }
@@ -4401,7 +4404,7 @@ network_interface_info (Lisp_Object ifname)
 
       for (it = ifap; it != NULL; it = it->ifa_next)
         {
-          struct sockaddr_dl *sdl = (struct sockaddr_dl *) it->ifa_addr;
+	  DECLARE_POINTER_ALIAS (sdl, struct sockaddr_dl, it->ifa_addr);
           unsigned char linkaddr[6];
           int n;
 
@@ -4722,7 +4725,7 @@ server_accept_connection (Lisp_Object server, int channel)
       {
 	args[nargs++] = procname_format_in6;
 	nargs++;
-	uint16_t *ip6 = (uint16_t *)&saddr.in6.sin6_addr;
+	DECLARE_POINTER_ALIAS (ip6, uint16_t, &saddr.in6.sin6_addr);
 	service = make_number (ntohs (saddr.in.sin_port));
 	for (int i = 0; i < 8; i++)
 	  args[nargs++] = make_number (ip6[i]);
-- 
2.9.4

Re: [PATCH] Fix use of sockaddr_in

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am Mi., 17. Mai 2017 um 22:38 Uhr:

> On 05/15/2017 02:04 AM, Philipp Stephani wrote:
> > - Maybe add verify(INT_MAX >= TYPE_MAXIMUM(in_port_t)) and
> > verify(TYPE_MINIMUM(in_port_t) == 0) to make sure that we can use int
> > for the port?
>
> The code has been changed so that the port number is no longer stored in
> an int so this remark is obsolete now. However, if this issue crops up
> later I don't think we need to worry about it for IPv4 and IPv6 ports,
> since they're always in the range 0..65535. Emacs already assumes that
> 'int' is at least 32 bits wide, as per POSIX.
>

Yes, these static assertions are more for documenting the assumptions that
the code makes in a way that actually checks the assumptions.


>
> > - Should there be a guard against the alias violation, e.g. by
> > declaring sa and sa1 with __attribute__((may_alias))? Otherwise it's
> > UB and the compiler might elide the switch entirely.
>
> Yes, that is easy enough to do and would avoid some unlikely but
> hard-to-catch bugs. I installed the attached. Most likely other parts of
> Emacs should use may_alias too; do you happen to have any tool in your
> toolbox that would find them systematically?
>
>
A type-based aliasing sanitizer is currently being added to Clang. Once
that is landed I'll run it over the Emacs codebase.

[-- Attachment #2: Type: text/html, Size: 1884 bytes --]

Re: [PATCH] Fix use of sockaddr_in

[Eli Zaretskii]

> Cc: phst@google.com, p.stephani2@gmail.com, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 14 May 2017 23:15:46 -0700
> 
> Eli Zaretskii wrote:
> > if your
> > intent was to say that this condition should be added to the code,
> > i.e. for AF_UNIX we should not try to compute the port, then I agree.
> 
> I installed a modified version of Philipp's patch, to do that. Thanks, Philipp, 
> for tracking this down.

I wonder why you ignored what I said here:

  http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00361.html

If you disagree, I'd expect you to say so and try to convince, not to
go ahead and commit the code.

Re: [PATCH] Fix use of sockaddr_in

[Paul Eggert]

On 05/17/2017 08:16 AM, Eli Zaretskii wrote:
> I wonder why you ignored what I said here:
>
>    http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00361.html

I had forgotten about that comment. And it took such an odd approach 
("Oh, let's refuse to run on any implementation that has an unusual 
struct sockaddr layout, even though it's easy to write portable code 
that works well") that it shouldn't be surprising that I forgot about 
it. However, you evidently prefer the odd approach so we'll try things 
your way.

Re: [PATCH] Fix use of sockaddr_in

[Lars Ingebrigtsen]

Philipp Stephani <p.stephani2@gmail.com> writes:

> +                    case AF_INET:
> +                      port = ((struct sockaddr_in6 *) (struct sockaddr *) &sa1)->sin6_port;
> +                      has_port = true;
> +                      break;
> +                    case AF_INET6:
> +                      port = ((struct sockaddr_in *) (struct sockaddr *) &sa1)->sin_port;
> +                      has_port = true;

Aren't these two cases in reverse?  If it's AF_INET6, it's an in6
struct, not the other way around.

Not that it matters, since (as Eli said) the sizes of the first elements
in the structs are identical...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [PATCH] Fix use of sockaddr_in

[Philipp Stephani]

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

Lars Ingebrigtsen <larsi@gnus.org> schrieb am So., 14. Mai 2017 um
12:28 Uhr:

> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> > +                    case AF_INET:
> > +                      port = ((struct sockaddr_in6 *) (struct sockaddr
> *) &sa1)->sin6_port;
> > +                      has_port = true;
> > +                      break;
> > +                    case AF_INET6:
> > +                      port = ((struct sockaddr_in *) (struct sockaddr
> *) &sa1)->sin_port;
> > +                      has_port = true;
>
> Aren't these two cases in reverse?  If it's AF_INET6, it's an in6
> struct, not the other way around.
>

Oops, thanks!


>
> Not that it matters, since (as Eli said) the sizes of the first elements
> in the structs are identical...
>
>
I don't know whether we can rely on that (i.e. whether Posix guarantees it)
or whether it's an implementation detail.
(This also needs __attribute__((may_alias)), type punning, or memcpy due to
aliasing.)

[-- Attachment #2: Type: text/html, Size: 1640 bytes --]