Thanks for working on this.

In D182546#6031157, @jonco wrote:

It might be nicer to continue to use JS_DECLARE_NEW_METHODS for js_new but it's more important to fix the problem without making more sweeping changes to the codebase. All this code could use some careful refactoring anyway.

Thanks for the fix.

I don't have write access to mozilla-central, so I need some help to land it.

And I see it's "blocked" for "no author info" because I've not used hg for generating the patch. Is it possible to work it around or I really need to clone the large hg repository and regenerate the patch?

In D182546#6031161, @xry111 wrote:

I don't have write access to mozilla-central, so I need some help to land it.

I can push this for you next week (there's a code freeze right now).

And I see it's "blocked" for "no author info" because I've not used hg for generating the patch. Is it possible to work it around or I really need to clone the large hg repository and regenerate the patch?

I can't seem to change that. You could ask in #developers in case anyone knows.

In D182546#6031174, @jonco wrote:

In D182546#6031161, @xry111 wrote:

I don't have write access to mozilla-central, so I need some help to land it.

I can push this for you next week (there's a code freeze right now).

And I see it's "blocked" for "no author info" because I've not used hg for generating the patch. Is it possible to work it around or I really need to clone the large hg repository and regenerate the patch?

I can't seem to change that. You could ask in #developers in case anyone knows.

Then I'll try hg in the weekend, and I can add a message to static_assert by the way.

Is the branch opened now?

In D182546#6044284, @xry111 wrote:

Is the branch opened now?

Sorry for the delay.

Code analysis found 4 defects in diff 740188:

• 1 defect found by clang-format
• 3 defects found by clang-format (Mozlint)

You can run this analysis locally with:

• ./mach lint --warnings --outgoing
• ./mach clang-format -p js/public/Utility.h

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 740188.

Code analysis found 3 defects in diff 740195:

• 3 defects found by clang-format (Mozlint)

4 defects closed compared to the previous diff 740188.

You can run this analysis locally with:

• ./mach lint --warnings --outgoing

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 740195.

3 defects closed compared to the previous diff 740195.

If you see a problem in this automated review, please report it here.

Thanks for the updates!

I pushed this to try again but there are some failures during asan tests: https://treeherder.mozilla.org/logviewer?job_id=422108005&repo=try&lineNumber=2016

I'm not exactly sure what's happening here but it may be due to how asan is set up.

In D182546#6051225, @jonco wrote:

Thanks for the updates!

I pushed this to try again but there are some failures during asan tests: https://treeherder.mozilla.org/logviewer?job_id=422108005&repo=try&lineNumber=2016

I'm not exactly sure what's happening here but it may be due to how asan is set up.

Ooooops... From the man page of memalign:

POSIX requires that memory obtained from posix_memalign() can be freed using free(3). Some systems provide no way to reclaim memory allocated with memalign() or valloc() (because one can pass to free(3) only a pointer obtained from malloc(3), while, for example, memalign() would call malloc(3) and then align the obtained value). The glibc implementation allows memory obtained from any of these functions to be reclaimed with free(3).

I guess Windows is a platform where you cannot free the pointer returned by memalign :(. I'll try to figure out a solution.

We have:

#ifndef HAVE_MEMALIGN
MOZ_MEMORY_API void* memalign(size_t aAlignment, size_t aSize) {
#  ifdef XP_WIN
  return _aligned_malloc(aSize, aAlignment);
#  else
  void* ret;
  if (posix_memalign(&ret, aAlignment, aSize) != 0) {
    return nullptr;
  }
  return ret;
#  endif
}
#endif

but Microsoft explicitly says we cannot free() a pointer returned by _aligned_malloc.

That seems wrong... do you want to file a bug for that?

In D182546#6051325, @jonco wrote:

That seems wrong... do you want to file a bug for that?

https://bugzilla.mozilla.org/show_bug.cgi?id=1842582

In the meantime maybe I should try to make GCRuntime no longer over-aligned instead...

Alright, so I guess we should just remove the over-alignment of js::Nursery and js::GCMarker for now. Anyway js_new does not guarantee the alignment for them, so let's stop "pretending" they were aligned.

Code analysis found 6 defects in diff 740386:

• 6 defects found by clang-format (Mozlint)

You can run this analysis locally with:

• ./mach lint --warnings --outgoing

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 740386.

6 defects closed compared to the previous diff 740386.

If you see a problem in this automated review, please report it here.