1249221 - Investigate ssl3con.c address changes

Status: RESOLVED WORKSFORME

(NSS :: Libraries, defect)

Description

[Franziskus Kiefer [:franziskus]]

Adding line breaks in ssl3con.c makes the resulting binaries differ in multiple places.

For example changing
> case hello_request:         rv = "hello_request (0)";               break;
to
> case hello_request:
>            rv = "hello_request (0)";
>            break;
makes the resulting binary change addresses such as
> 41 b8 98 2b 00 00    	mov    $0x2b98,%r8d
VS
> 41 b8 99 2b 00 00    	mov    $0x2b99,%r8d

Comment 1

[Ryan Sleevi]

Is that the *only* change, or is that just an *example* change?

We've already discussed issues like this in the past, where it ended up being changes for macros / line numbers / function names. Also, is this differences in an optimized build or a debug build?

Comment 2

[Kai Engert (:KaiE:)]

(In reply to Ryan Sleevi from comment #1)
> Is that the *only* change, or is that just an *example* change?

He said on IRC:

"I always get 4 different addresses after changing any layout in ssl3con. none of them are switched, they are just different to the ones before but then deterministic again"


> We've already discussed issues like this in the past, where it ended up
> being changes for macros / line numbers / function names. Also, is this
> differences in an optimized build or a debug build?

Optimized build he said.

Comment 3

[Ryan Sleevi]

It'd be curious to also know if running strings + diff show a difference

Comment 4

[Franziskus Kiefer [:franziskus]]

yeah, everything that Kai said.

(In reply to Ryan Sleevi from comment #3)
> It'd be curious to also know if running strings + diff show a difference

Just tried that. strings don't show differences.

Comment 5

[Kai Engert (:KaiE:)]

In all 4 affected places, we have code like this:
   mov    $0x2cc4,%r8d

If I understand correctly, source is left, destination is right.

A double word from some memory location is copied to register 8.

During the remainder of that function, I cannot find any other instructions that use r8 as a source.

This applies to all 4 places that differ between the binaries. Each time, the r8 doesn't get mentioned again.

In on occassion, two statements like the above happen in a single function. Between the first assignment to r8d and the second assignment to r8d, I also cannot see any other use of r8.

I know very little about assembler programming, so I don't know if there are any instructions that might make use of r8 without mentioning it in the assembler instruction.

I will attach an objdump -S -x disassembly of that function, so someone else with better assembler skills might doublecheck, if that value gets ever used after moving it to r8.

Comment 6

[Kai Engert (:KaiE:)]

Attachment: disas.txt

Comment 7

[Kai Engert (:KaiE:)]

I recommend that we attempt to check in the reformatting, and see if the test suite discovers anything.

Since NSS 3.23 and Firefox 45 aren't expected to be in a final release until 2016-06-07, I think we should have enough time to discover regressions.

Comment 8

[Franziskus Kiefer [:franziskus]]

Hi Nathan, I was told you might know compiler things and could know what's happening here (see comment 5 and comment 0 for details).

Comment 9

[Nathan Froyd [:froydnj]]

In Kai's disassembly from comment 6, we have the following lines called out with a "->", with surrounding code for context:

   10408:	48 8d 0d 91 49 02 00 	lea    0x24991(%rip),%rcx        # 34da0 <nonDTLSSuites+0x180>
   1040f:	48 8d 15 2a 4d 02 00 	lea    0x24d2a(%rip),%rdx        # 35140 <__func__.16684>
-> 10416:	41 b8 a8 2c 00 00    	mov    $0x2ca8,%r8d
   1041c:	be 03 00 00 00       	mov    $0x3,%esi
   10421:	48 89 df             	mov    %rbx,%rdi
   10424:	e8 b7 21 02 00       	callq  325e0 <tls13_SetHsState>

   10408:	48 8d 0d 91 49 02 00 	lea    0x24991(%rip),%rcx        # 34da0 <nonDTLSSuites+0x180>
   1040f:	48 8d 15 2a 4d 02 00 	lea    0x24d2a(%rip),%rdx        # 35140 <__func__.16684>
-> 10416:	41 b8 a8 2c 00 00    	mov    $0x2ca8,%r8d
   1041c:	be 03 00 00 00       	mov    $0x3,%esi
   10421:	48 89 df             	mov    %rbx,%rdi
   10424:	e8 b7 21 02 00       	callq  325e0 <tls13_SetHsState>

The lines of code prior to the |callq| instructions in both instances are setting up argument registers for a call to tls13_SetHsState.  The registers used for argument passing on x86-64 are %rdi, %rsi, %rdx, %rcx, %r8, and %r9, in that order.  tls13_SetHsState has the following prototype:

void tls13_SetHsState(sslSocket *ss, SSL3WaitState ws,
                      const char *func, const char *file, int line)

In the case of tls13_SetHsState, %r8 would hold the |line| argument.  Looks like reformatting changes the __LINE__ parameter that gets passed to tls13_SetHsState via the TLS13_SET_HS_STATE macro in tls13con.h.  And indeed, ssl3_AuthCertificate, which is the function that Kai disassembled, contains two calls to TLS13_SET_HS_STATE.

Comment 10

[Kai Engert (:KaiE:)]

Nathan, thank you very much for your explanations.

That means the changes are safe, because the information is simply used for line numbers in tracing.

I wonder if we should use a different version of the macros and functions, which avoids embedding the debug information in the binary, when building optimized builds.