--enable-pie should support clang

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tjr, Assigned: Alex_Gaynor)

Tracking

(Blocks: 1 bug, {sec-want})

Trunk
mozilla55
sec-want
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [sg:want][adv-main55-])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
This bug was created as a clone of Bug #620058 which has more context.
(Assignee)

Comment 1

2 years ago
https://dxr.mozilla.org/mozilla-central/source/build/autoconf/compiler-opts.m4#271 is the exact place that needs changing -- unless I've missed something this should be straightforward.
(Reporter)

Updated

2 years ago
No longer blocks: 1359918
(Reporter)

Updated

2 years ago
No longer depends on: 671426
(Assignee)

Updated

2 years ago
Assignee: nobody → agaynor
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8863468 - Flags: review?(nfroyd)
(Reporter)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8863468 [details]
Bug 1360300 -- Support enabling PIE when using clang.

https://reviewboard.mozilla.org/r/135254/#review138152

::: build/autoconf/compiler-opts.m4:267
(Diff revision 1)
>  MOZ_ARG_ENABLE_BOOL(pie,
>  [  --enable-pie           Enable Position Independent Executables],
>      MOZ_PIE=1,
>      MOZ_PIE= )
>  
> -if test "$GNU_CC" -a -n "$MOZ_PIE"; then
> +if test "$GNU_CC$CLANG_CC" -a -n "$MOZ_PIE"; then

Not sure if this will affect clang-cl, but thought I'd mention it...
(Assignee)

Comment 4

2 years ago
For future reference, here's how I verified that clang on Linux didn't have |-pie| by default:

root@7cc00b38b604:/# cat t.c
int main() {}
root@7cc00b38b604:/# clang t.c
root@7cc00b38b604:/# ./hardening-check a.out
a.out:
 Position Independent Executable: no, normal executable!
 Stack protected: no, not found!
 Stack protected: no, not found!
 Fortify Source functions: no, not found!
 Read-only relocations: yes
 Immediate binding: no, not found!
root@7cc00b38b604:/# clang -pie t.c
root@7cc00b38b604:/# ./hardening-check a.out
a.out:
 Position Independent Executable: yes
 Stack protected: no, not found!
 Stack protected: no, not found!
 Fortify Source functions: no, not found!
 Read-only relocations: yes
 Immediate binding: no, not found!
root@7cc00b38b604:/# clang --version
clang version 3.8.1-23 (tags/RELEASE_381/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin


(This is using a Debian stretch docker container, do not be alarmed by root :-))
(Assignee)

Comment 5

2 years ago
mozreview-review-reply
Comment on attachment 8863468 [details]
Bug 1360300 -- Support enabling PIE when using clang.

https://reviewboard.mozilla.org/r/135254/#review138152

> Not sure if this will affect clang-cl, but thought I'd mention it...

https://clang.llvm.org/docs/UsersManual.html#clang-cl doesn't list it; all the flags listed are MSVC syle `/flag`. It didn't support `-fstack-protector-strong` so it makes sense to me that it's not supported here either.

Comment 6

2 years ago
mozreview-review
Comment on attachment 8863468 [details]
Bug 1360300 -- Support enabling PIE when using clang.

https://reviewboard.mozilla.org/r/135254/#review138158
Attachment #8863468 - Flags: review?(nfroyd) → review+

Comment 7

2 years ago
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #4)
> For future reference, here's how I verified that clang on Linux didn't have
> |-pie| by default:

By the way, on macOS this can be checked by 
$ otool -hv </path/to/executable>


$ otool -hv /Applications/Thunderbird.app/Contents/MacOS/thunderbird-bin 
Mach header
      magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
MH_MAGIC_64  X86_64        ALL LIB64     EXECUTE    23       2272   NOUNDEFS DYLDLINK TWOLEVEL BINDS_TO_WEAK PIE
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 9

2 years ago
mozreview-review
Comment on attachment 8863468 [details]
Bug 1360300 -- Support enabling PIE when using clang.

https://reviewboard.mozilla.org/r/135254/#review138172

::: commit-message-57b37:9
(Diff revisions 1 - 2)
>  comment isn't relevant.
>  
>  While PIE is enabled by default on macOS, this isn't true of clang on Linux.
>  --enable-pie can now be used with clang on Linux.
>  
> +r=froydnj

FWIW, if you write your commit message title as:

"Bug 1360300 - Support enabling PIE when using clang; r?froydnj"

mozreview will take care of flagging the correct reviewer when you push and updating the commit with the correct reviewer when it's landed, which will save you some steps.

Comment 10

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6087ce1bca59
Support enabling PIE when using clang. r=froydnj
Keywords: checkin-needed

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6087ce1bca59
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Whiteboard: [sg:want] → [sg:want][adv-main55-]
You need to log in before you can comment on or make changes to this bug.