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? On Wednesday, August 15, 2018 at 7:30:00 AM UTC-7, Jakob Kummerow wrote: > > [+v8-dev -chromium-dev] > > On Wed, Aug 15, 2018 at 2:42 PM Christian Biesinger <cbies...@chromium.org > <javascript:>> 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? > > >> > >> > 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! >> > >> > On Tuesday, August 14, 2018 at 5:21:32 PM UTC-7, stzh...@gmail.com >> wrote: >> >> >> >> 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 >> > >> > -- >> > -- >> > Chromium Developers mailing list: chromi...@chromium.org <javascript:> >> > 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 >> . >> >> -- >> -- >> Chromium Developers mailing list: chromi...@chromium.org <javascript:> >> 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/CAPTJ0XEemft9VzTgJ_ssfoWEzp6TEfCHx9vGjmeDHz%2BaQjmFWA%40mail.gmail.com >> . >> >> -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
