Unable to build avmshell with --enable-aot

RESOLVED FIXED

Status

Tamarin
Virtual Machine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Alok Manchanda, Unassigned)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Windows NT 5.1) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.100 Safari/534.30
Build Identifier: 

It appears this has broken again. Specific issues:
1) 'noreturn' function does return warning - treated as error
2) Include paths not set correctly when building AOT binaries
3) Initialization using {0} doesn't work for NativeMethodInfo
4) Unknown function VMPI_calloca_gc used under VMCFG_AOT


Reproducible: Always
(Reporter)

Comment 1

6 years ago
Created attachment 541343 [details] [diff] [review]
Proposed fix
Attachment #541343 - Flags: review?(stejohns)
(Reporter)

Comment 2

6 years ago
Steven - if the fix looks OK, would you please go ahead and submit it for me? Thanks!

Comment 3

6 years ago
Until AOT builds are set up as a regulat build on the Tamarin servers, this is going to continue to be a pain point.

Comment 4

6 years ago
Comment on attachment 541343 [details] [diff] [review]
Proposed fix

Review of attachment 541343 [details] [diff] [review]:
-----------------------------------------------------------------

Want me to land this for you?

::: aot/AOTStubs.cpp
@@ +110,5 @@
>      AvmAssert(AvmCore::isNullOrUndefined(atom));
>      // This could be made tighter by calling nullcheckfail(), but that's private and not really worth making public.
>      env->nullcheck(atom);
> +    // Appease g++, avoid 'noreturn' function does return, profit
> +    while(1) {};

style nit: ';' after the closing semi is unnecessary and some compilers will complain

::: core/ScopeChain.cpp
@@ +85,4 @@
>      /*static*/ const ScopeTypeChain* ScopeTypeChain::create(MMgc::GC* gc, Traits* traits, const ScopeTypeChain* outer, Traits* const* stateTraits, uint32_t nStateTraits, uint32_t nStateWithTraits, Traits* append, Traits* extra)
>      {
>          MMgc::GC::AllocaAutoPtr valuesPtr;
> +        FrameValue *values = (FrameValue *)VMPI_alloca_gc(gc, valuesPtr, sizeof(FrameValue)*nStateTraits);

Note that this API is about to get renamed, see https://bugzilla.mozilla.org/show_bug.cgi?id=664703
Attachment #541343 - Flags: review?(stejohns) → review+
(Reporter)

Comment 5

6 years ago
Created attachment 541642 [details] [diff] [review]
Proposed fix (removed ';' after while(1){}, used the new alloc function)

Steven, I've updated the patch - removed the ';' after the while loop and switched to the renamed alloc function. If this looks alright, please push it for me.
Thanks very much!
-alok.
Attachment #541642 - Flags: review?(stejohns)
(Reporter)

Comment 6

6 years ago
Yeah, after this fix I will start a conversation to add building avmshell with enable-aot to the automation runs.

(In reply to comment #3)
> Until AOT builds are set up as a regulat build on the Tamarin servers, this
> is going to continue to be a pain point.

Comment 7

6 years ago
Comment on attachment 541642 [details] [diff] [review]
Proposed fix (removed ';' after while(1){}, used the new alloc function)

Review of attachment 541642 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #541642 - Flags: review?(stejohns) → review+

Comment 8

6 years ago
pushed to TR 6426:fca98c77471f
Status: UNCONFIRMED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.