Closed Bug 1035786 Opened 10 years ago Closed 10 years ago

Build with "ac_add_options --enable-content-sandbox" is not compiling on Linux


(Core :: Security, defect)

30 Branch
Not set





(Reporter: julien.voisin, Assigned: julien.voisin)




(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140608211622

Steps to reproduce:

hg clone
echo "ac_add_options --enable-content-sandbox" > mozconfig
./mach build

Actual results:

1:28.44 SandboxAssembler.o
 1:28.81 In file included from ../../../dist/system_wrappers/prlog.h:3:0,
 1:28.81                  from /opt/mozilla-central/security/sandbox/linux/Sandbox.cpp:47:
 1:28.81 /opt/mozilla-central/security/sandbox/linux/Sandbox.cpp: In static member function ‘static void sandbox::Die::SandboxDie(const char*, const char*, int)’:
 1:28.81 /opt/mozilla-central/security/sandbox/linux/Sandbox.cpp:58:35: error: ‘gSeccompSandboxLog’ was not declared in this scope
 1:28.81  #define LOG_ERROR(args...) PR_LOG(gSeccompSandboxLog, PR_LOG_ERROR, (args))

Expected results:

A successful compilation.
Attachment #8452268 - Flags: review?(fbraun)
I haven't worked on sandbox, according to 'hg blame', :kang or :jld could be of help.
Can you guys take a look at this patch?
Flags: needinfo?(jld)
Flags: needinfo?(gdestuynder)
Attachment #8452268 - Flags: review?(fbraun)
Component: Untriaged → Security
Flags: needinfo?(jld)
Product: Firefox → Core
As it happens, after my conversation with jvoisin on IRC, I needed to compile this locally on linux while testing bug 1035275.

I hit the same error and this patch certainly allows the compile to complete.
Whether it's the best fix, I'm not sure.

On try we also appear to be treating some warnings as errors on some builds (that my local build doesn't).

Here are try pushes without and with this patch (there are patches from other bugs I'm working on in there, but I don't think they're affecting this compile):
Ever confirmed: true
Summary: --enable-content-sanbox is not compiling → Build with "ac_add_options --enable-content-sandbox" is not compiling on Linux
Here's another patch to fix the member variable initialisation order, still getting other problems on Try though:
Flags: needinfo?(gdestuynder)
Assignee: nobody → bobowencode
(In reply to Bob Owen (:bobowen) from comment #3)
> Here's another patch to fix the member variable initialisation order, still
> getting other problems on Try though:

Explicitly casting getpid() should be safe, because the value won't be negative.

For the missing syscalls on non-x86_64 architectures, I have a patch that should take care of it; I'll file a separate bug.
I've converted the diff to Mercurial export/import format so it can be checked in[*] — I added a commit message and filled in the author information from your bugzilla user info.  If you'd like something different there, please feel free to edit the patch and reupload it.

Attachment #8452268 - Attachment is obsolete: true
Attachment #8454208 - Flags: review+
Attachment #8454208 - Flags: feedback?(julien.voisin)
Thanks Jed,

I think jvoisin should be the assignee here as he found and fixed the original problem.
Assignee: bobowencode → julien.voisin
Fine for me. What do I need to do to get it merged?
Attachment #8454208 - Flags: feedback?(julien.voisin) → feedback+
Comment on attachment 8454200 [details] [diff] [review]
Avoid warning-as-error sandbox build failure with an explicit cast.

Review of attachment 8454200 [details] [diff] [review]:

Note that while is this safe because maxpid defaults to 32768 and is always 0 or positive, IIRC pid_t is a signed 32bit integer (/usr/include/bits/typesizes.h for ex.)
Attachment #8454200 - Flags: review?(gdestuynder) → review+
You need to log in before you can comment on or make changes to this bug.