Best way to add a custom flag (bool) in v8::internal::JSFunction?

[stzh...@gmail.com]

Hi all,

I've been trying hard to add a custom bool flag in v8::internal::JSFunction. It feels like classes like JSFunction are not designed for adding things this way because I ran into all kinds of errors and/or DCHECK failures.

Methods I've tried so far: (i) adding bool variables directly to v8/src/objects.h and v8/src/objects-inl.h directly ---> object layout gets garbled so it's no longer valid executable JSFunction; (ii) adding fields via DECL_BOOLEAN_ACCESSORS and other macros ---> various bits seem to need adjustment, otherwise fails DCHECKs. Without detailed documentation, I'm basically in a haze in terms of how should I re-calculate those bits.

Any tip or working example of adding such a field? Existing examples in the code look pretty confusing as they don't seem to have a clear documentation/usage (per my understanding of Chromium internals, which is quite basic...). Thanks a lot!

Best,

- Z

[stzh...@gmail.com]

Although I don't know how close I got, this is my latest attempt:

in shared-function-info.h:

......

DECL_INT_ACCESSORS(flags2)

......

DECL_BOOLEAN_ACCESSORS(is_foo)

......

// Layout description.

#define SHARED_FUNCTION_INFO_FIELDS(V) \

/* Pointer fields. */ \

V(kStartOfPointerFieldsOffset, 0) \

V(kFunctionDataOffset, kPointerSize) \

V(kNameOrScopeInfoOffset, kPointerSize) \

V(kOuterScopeInfoOrFeedbackMetadataOffset, kPointerSize) \

V(kScriptOffset, kPointerSize) \

V(kFunctionIdentifierOrDebugInfoOffset, kPointerSize) \

V(kEndOfPointerFieldsOffset, 0) \

/* Raw data fields. */ \

V(kUniqueIdOffset, kUniqueIdFieldSize) \

V(kLengthOffset, kUInt16Size) \

V(kFormalParameterCountOffset, kUInt16Size) \

V(kExpectedNofPropertiesOffset, kUInt16Size) \

V(kFunctionTokenOffsetOffset, kUInt16Size) \

V(kFlagsOffset, kInt32Size) \

V(kFlagsOffset2, kInt32Size) \

/* Total size. */ \

V(kSize, 0)

......

// Bit positions in |flags2|

#define FLAGS_BIT_FIELDS2(V, _) \

V(IsFooBit, bool, 1, _)

DEFINE_BIT_FIELDS(FLAGS_BIT_FIELDS2)

#undef FLAGS_BIT_FIELDS2

in shared-fucntion-info-inl.h

BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags2, is_foo,

SharedFunctionInfo::IsFooBit)

Also I exposed proper setter/getter(s) in v8.h and api.cc. However, I keep getting errors while loading pages:

abort: The function_data field should be a BytecodeArray on interpreter entry

Received signal 11 SEGV_MAPERR 0009ffffffff

#0 0x7fd8eff58d5d base::debug::StackTrace::StackTrace()

#1 0x7fd8efc3cdec base::debug::StackTrace::StackTrace()

#2 0x7fd8eff587b4 base::debug::(anonymous namespace)::StackDumpSignalHandler()

#3 0x7fd8d0fbe390 <unknown>

#4 0x7fd8dad4068f v8::internal::AbstractCode::SourcePosition()

#5 0x7fd8da67c4ce v8::internal::JavaScriptFrame::Print()

#6 0x7fd8da770f81 v8::internal::Isolate::PrintStack()

#7 0x7fd8da772b37 v8::internal::Isolate::PrintStack()

#8 0x7fd8da9a2c16 v8::internal::__RT_impl_Runtime_Abort()

#9 0x7fd8da9a28df v8::internal::Runtime_Abort()

I know I'm now playing with some executable HeapObject at runtime so I need to make sure its layout is valid. Having this said, it remains unclear to me what I should do exactly to adjust relevant bits to ensure the JSFunction object pertains its recognizable structure. Could anyone please provide a little input on this? Thanks so much in advance!

[Christian Biesinger]

You may be better off asking on the V8 mailing list:
https://groups.google.com/group/v8-dev

Christian

> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
> ---
> You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/84f9fc7b-320e-4fb4-9d8c-314e274e057e%40chromium.org.

[Jakob Kummerow]

[+v8-dev -chromium-dev]

On Wed, Aug 15, 2018 at 2:42 PM Christian Biesinger <cbies...@chromium.org> wrote:

You may be better off asking on the V8 mailing list:
https://groups.google.com/group/v8-dev

Yup, redirecting there.

> Although I don't know how close I got, this is my latest attempt:

That looks fine.

That said, these are changes to SharedFunctionInfo, whereas in your mail, you talk about adding a field to JSFunction. Which did you do?

The patch looks fine, the error is surprising. Adding a field at the end of an object shouldn't perturb other fields at its beginning... Are you sure you haven't made any other changes?

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAPTJ0XEemft9VzTgJ_ssfoWEzp6TEfCHx9vGjmeDHz%2BaQjmFWA%40mail.gmail.com.

[stzh...@gmail.com]

Hi Jakob,

Sorry that I didn't follow up in in email thread. I was just a bit worried for taking too much of your time there... :)

OK, so per this new patch, 

That said, these are changes to SharedFunctionInfo, whereas in your mail, you talk about adding a field to JSFunction. Which did you do?

So yes I switched back to modifying SharedFunctionInfo, because I realized adding extra fields in v8::internal::JSFunction might be too costly as it's been passed around very frequently at runtime (vs SharedFunctionInfo is shared among all derived closures, right?). You mentioned before it's impossible to add any more field to the existing |flags| because it's already full, which is why I declared the 2nd BIT_FIELD with |flags2| and kFlagsOffset2 so I can have some additional space in the object.

The patch looks fine, the error is surprising. Adding a field at the end of an object shouldn't perturb other fields at its beginning... Are you sure you haven't made any other changes?

I believe these are all changes I made in the scope of SharedFunctionInfo. In fact after undef'ing the patch I can run Chromium smoothly. I too think the addition at the end of an object shouldn't interfere with fields at the beginning. Is there anything else I need to take care of? I searched hard but didn't any other macros/functions that are required to invoke for adding a field. Wonder if anything there for "repacking" the function_data?