1432790 - Sandbox MinGW Compilation error: undefined reference to `_ReturnAddress'

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Tom Ritter [:tjr]]

In Bug 1431621 I thought I had fixed it by calling mingw's _ReturnAddress() function, but this caused an error on the linking step (below).  _ReturnAddress() was just a #define in MinGW to __builtin_return_address - so I'm just going to restore the original code but fix the stray \

[task 2018-01-24T00:44:34.562Z] 00:44:34     INFO -  ../../security/sandbox/process_handle_win.o: In function `ZN4base3win19GenericScopedHandleINS0_12HandleTraitsENS0_14VerifierTraitsEE3SetEPv':
[task 2018-01-24T00:44:34.562Z] 00:44:34     INFO -  /builds/worker/workspace/build/src/security/sandbox/chromium/base/win/scoped_handle.h:70: undefined reference to `_ReturnAddress'
[task 2018-01-24T00:44:34.562Z] 00:44:34     INFO -  ../../security/sandbox/process_handle_win.o: In function `ZN4base3win19GenericScopedHandleINS0_12HandleTraitsENS0_14VerifierTraitsEE5CloseEv':
[task 2018-01-24T00:44:34.563Z] 00:44:34     INFO -  /builds/worker/workspace/build/src/security/sandbox/chromium/base/win/scoped_handle.h:95: undefined reference to `_ReturnAddress'

(This does compile btw)

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1432790 Fix MinGW Sandbox linking error about not being able to find _ReturnAddress()

This reverts 6e41201152dd (Bug 1431621) which compiled but did not link.
It also fixes the original issue by removing the stray \ at the end of the
line that was causing the error.

Review commit: https://reviewboard.mozilla.org/r/215302/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/215302/

Comment 2

[Tom Ritter [:tjr]]

Comment on attachment 8945080 [details]
Bug 1432790 Fix MinGW Sandbox linking error about not being able to find _ReturnAddress()

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215302/diff/1-2/

Comment 3

[Bob Owen (:bobowen) (PTO until 17th Apr)]

Comment on attachment 8945080 [details]
Bug 1432790 Fix MinGW Sandbox linking error about not being able to find _ReturnAddress()

https://reviewboard.mozilla.org/r/215302/#review220878

::: security/sandbox/chromium-shim/patches/with_update/mingw_base_win_get_caller.patch:7
(Diff revision 2)
> -Bug 1431621 Remove the gcc variant of BASE_WIN_GET_CALLER, because MinGW defines _ReturnAddress() ?rbobowen
> +Bug XXXX Fix a stray \ in MinGW code
>  
> -Note that MinGW defines it without __builtin_extract_return_addr which
> -means we're dropping that, but the gcc documentation indicates that
> +This is a bastardized pseudo-commit of Bug 1431621 and Bug 1432790.
> +It removes a stray \ in the code that causes MinGW to not compile.

Please make this just be a commit message (and patch) as if it had been done like this from the start for bug 1431621.

::: security/sandbox/chromium-shim/patches/with_update/mingw_base_win_get_caller.patch:19
(Diff revision 2)
>  MozReview-Commit-ID: 4D5bIT9Fei4
>  
>  diff --git a/security/sandbox/chromium/base/win/scoped_handle.h b/security/sandbox/chromium/base/win/scoped_handle.h
>  --- a/security/sandbox/chromium/base/win/scoped_handle.h
>  +++ b/security/sandbox/chromium/base/win/scoped_handle.h
>  @@ -1,31 +1,26 @@

@@ -13,17 +13,17 @@

::: security/sandbox/chromium-shim/patches/with_update/mingw_base_win_get_caller.patch:20
(Diff revision 2)
>   // Copyright (c) 2012 The Chromium Authors. All rights reserved.
>   // Use of this source code is governed by a BSD-style license that can be
>   // found in the LICENSE file.
>   
>   #ifndef BASE_WIN_SCOPED_HANDLE_H_
>   #define BASE_WIN_SCOPED_HANDLE_H_
>   
> -+#include <intrin.h>
>   #include <windows.h>
>   
>   #include "base/base_export.h"
>   #include "base/gtest_prod_util.h"
>   #include "base/location.h"

Delete these ...

::: security/sandbox/chromium-shim/patches/with_update/mingw_base_win_get_caller.patch:50
(Diff revision 2)
>   // the ScopedFILE class with two additions:
>   //   - IsValid() method can tolerate multiple invalid handle values such as NULL

... and these.

Comment 4

[Tom Ritter [:tjr]]

Comment on attachment 8945080 [details]
Bug 1432790 Fix MinGW Sandbox linking error about not being able to find _ReturnAddress()

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215302/diff/2-3/

Comment 5

[Tom Ritter [:tjr]]

Okay I think I corrected it. I generated the patch file using real hg commands instead of editing it manually, but I guess I'd prefer you to double check it before I commit it.

Comment 6

[Bob Owen (:bobowen) (PTO until 17th Apr)]

Yes, looks good to me, thanks.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/112289b0c784
Fix MinGW Sandbox linking error about not being able to find _ReturnAddress() r=bobowen

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/112289b0c784