Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build and linking issue on 64-bit #4

Merged
merged 1 commit into from
May 24, 2018

Conversation

KapiX
Copy link
Member

@KapiX KapiX commented May 22, 2018

Comments about -pie flag:
I'm guessing WebKit/WebKit@8b224d4 broke linking when it switched to CMAKE_POSITION_INDEPENDENT_CODE.
Haiku's CMake platform file does not have PIC/PIE flags defined (removed here Kitware/CMake@7ebc1cb#diff-6a683ecc2f7a61f1096c7918bfd12a70) although they are correctly added during build when compiling source files. It's linking that fails and -pie is a linker flag.
Also CMake's test for Position Independent Executables passes.
The only platform that has it defined is Android:
https://github.com/Kitware/CMake/search?utf8=%E2%9C%93&q=-pie&type=

Build tested with 64-bit GCC 7.3. Please test if it builds on 32-bit.

@pulkomandy
Copy link
Member

github search seems unreliable on big repos, for me it does not see anything in Android, but finds something for SunOS.

So I guess we have a problem with our CMake port (the WebKit commit message does mention "cmake adds the flag for us").

@KapiX
Copy link
Member Author

KapiX commented May 22, 2018

For me it shows three results, and android one is second:
https://github.com/Kitware/CMake/blob/4e23a1d0eb0eca303d28c3f5b7b7147739f891b3/Modules/Platform/Android/abi-common.cmake#L16

Anyway, why do we need -pie for linker in the first place? As I mentioned, -fPIC and -fPIE flags are added correctly for source files, and PIE unit test in CMake's test suite passes.

@pulkomandy
Copy link
Member

Ok, so let's dig into cmake.

According to the docs, what CMAKE_POSITION_INDEPENDENT_CODE does is set the default value for POSITION_INDEPENDENT_CODE, which can also be set per-target. So the Android handling seems suspicious.

If we dig into POSITION_INDEPENDENT_CODE, we can see it is implemented here: https://github.com/Kitware/CMake/blob/d4b97e8d721e5a5374a400bcdd5df1eb4e1cf7d2/Source/cmLocalGenerator.cxx#L1783 by adding the "position independant flags". This (https://github.com/Kitware/CMake/blob/d4b97e8d721e5a5374a400bcdd5df1eb4e1cf7d2/Source/cmLocalGenerator.cxx#L1832) in turn looks for CMAKE_CXX_COMPILE_OPTIONS_PIE and _PIC. Clearly the Haiku platform file forgets to set these: https://github.com/Kitware/CMake/search?p=1&q=CMAKE_C_COMPILE_OPTIONS_PIE&type=&utf8=✓ (possibly they are not needed on x86 and/or gcc2 in our case beause of tweaks in our gcc port).

@KapiX
Copy link
Member Author

KapiX commented May 22, 2018

I'd still go with our GCC being broken. Compare:
https://github.com/gcc-mirror/gcc/blob/da8dff89fa9398f04b107e388cb706517ced9505/gcc/config/i386/gnu-user64.h#L67
with
https://github.com/haikuports/haikuports/blob/master/sys-devel/gcc/patches/gcc-5.4.0_2016_06_04.patchset#L838
and
https://github.com/haikuports/haikuports/blob/master/sys-devel/gcc/patches/gcc-7.3.0_2018_05_01.patchset#L906

EDIT: admittedly, -pie is not in 7.3.0, and it is for static-pie target, but I think this flag is supposed to be passed implicitily, and fixing this in CMake is just a workaround.

@pulkomandy
Copy link
Member

-fPIC is enabled by default for our GCC (if I'm not mistaken, it happens by redefining CC1_SPEC). We do this because our executables are always shared objects, and can be loaded as add-ons into other executables (at least for 32bit systems, maybe we do something different for 64bit). This can explain the difference on gcc side (but it's quite possible something isn't done right there too).

@KapiX
Copy link
Member Author

KapiX commented May 22, 2018

Yes, and if I understand correctly there is equivalent LINK_SPEC which I think should contain -pie, but currently it doesn't.

@pulkomandy pulkomandy merged commit 5e325ab into haiku:rebased May 24, 2018
@nielx
Copy link
Member

nielx commented Jun 2, 2018

I've changed this line of the LINK_SPEC from elf_x86_64_haiku to elf_x86_64, and this did not make a difference when compiling a 32-bit cross-compiler from 64 bit Haiku. I'm wondering if that's actually the change you were referring to...

pulkomandy pushed a commit that referenced this pull request Dec 5, 2018
…llFrame.

https://bugs.webkit.org/show_bug.cgi?id=191579
<rdar://problem/45942472>

Reviewed by Saam Barati.

JSTests:

* stress/regress-191579.js: Added.

Source/JavaScriptCore:

Both of these functions do a lot of work.  It would be good for the topCallFrame
to be correct should we need to throw an exception.

For example, we've observed the following crash trace:

  * frame #0: WTFCrash() at Assertions.cpp:253
    frame #1: ...
    frame #2: JSC::StructureIDTable::get(this=0x00006040000162f0, structureID=1874583248) at StructureIDTable.h:129
    frame #3: JSC::VM::getStructure(this=0x0000604000016210, id=4022066896) at VM.h:705
    frame #4: JSC::JSCell::structure(this=0x00007ffeefbbde30, vm=0x0000604000016210) const at JSCellInlines.h:125
    frame #5: JSC::JSCell::classInfo(this=0x00007ffeefbbde30, vm=0x0000604000016210) const at JSCellInlines.h:335
    frame #6: JSC::JSCell::inherits(this=0x00007ffeefbbde30, vm=0x0000604000016210, info=0x0000000105eaf020) const at JSCellInlines.h:302
    frame #7: JSC::JSObject* JSC::jsCast<JSC::JSObject*, JSC::JSCell>(from=0x00007ffeefbbde30) at JSCast.h:36
    frame #8: JSC::asObject(cell=0x00007ffeefbbde30) at JSObject.h:1299
    frame #9: JSC::asObject(value=JSValue @ 0x00007ffeefbba380) at JSObject.h:1304
    frame #10: JSC::Register::object(this=0x00007ffeefbbdd58) const at JSObject.h:1514
    frame #11: JSC::ExecState::jsCallee(this=0x00007ffeefbbdd40) const at CallFrame.h:107
    frame #12: JSC::ExecState::isStackOverflowFrame(this=0x00007ffeefbbdd40) const at CallFrameInlines.h:36
    frame #13: JSC::StackVisitor::StackVisitor(this=0x00007ffeefbba860, startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800) at StackVisitor.cpp:52
    frame #14: JSC::StackVisitor::StackVisitor(this=0x00007ffeefbba860, startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800) at StackVisitor.cpp:41
    frame #15: void JSC::StackVisitor::visit<(JSC::StackVisitor::EmptyEntryFrameAction)0, JSC::Interpreter::getStackTrace(JSC::JSCell*, WTF::Vector<JSC::StackFrame, 0ul, WTF::CrashOnOverflow, 16ul>&, unsigned long, unsigned long)::$_3>(startFrame=0x00007ffeefbbdd40, vm=0x0000631000000800, functor=0x00007ffeefbbaa60)::$_3 const&) at StackVisitor.h:147
    frame #16: JSC::Interpreter::getStackTrace(this=0x0000602000005db0, owner=0x000062d00020cbe0, results=0x00006020000249d0, framesToSkip=0, maxStackSize=1) at Interpreter.cpp:437
    frame #17: JSC::getStackTrace(exec=0x000062d00002c048, vm=0x0000631000000800, obj=0x000062d00020cbe0, useCurrentFrame=true) at Error.cpp:170
    frame #18: JSC::ErrorInstance::finishCreation(this=0x000062d00020cbe0, exec=0x000062d00002c048, vm=0x0000631000000800, message=0x00007ffeefbbb800, useCurrentFrame=true) at ErrorInstance.cpp:119
    frame #19: JSC::ErrorInstance::create(exec=0x000062d00002c048, vm=0x0000631000000800, structure=0x000062d0000f5730, message=0x00007ffeefbbb800, appender=0x0000000000000000, type=TypeNothing, useCurrentFrame=true)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred), JSC::RuntimeType, bool) at ErrorInstance.h:49
    frame #20: JSC::createRangeError(exec=0x000062d00002c048, globalObject=0x000062d00002c000, message=0x00007ffeefbbb800, appender=0x0000000000000000)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred)) at Error.cpp:68
    frame #21: JSC::createRangeError(exec=0x000062d00002c048, globalObject=0x000062d00002c000, message=0x00007ffeefbbb800) at Error.cpp:316
    frame #22: JSC::createStackOverflowError(exec=0x000062d00002c048, globalObject=0x000062d00002c000) at ExceptionHelpers.cpp:77
    frame #23: JSC::createStackOverflowError(exec=0x000062d00002c048) at ExceptionHelpers.cpp:72
    frame #24: JSC::throwStackOverflowError(exec=0x000062d00002c048, scope=0x00007ffeefbbbaa0) at ExceptionHelpers.cpp:335
    frame #25: JSC::ProxyObject::getOwnPropertySlotCommon(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbba80, slot=0x00007ffeefbbc720) at ProxyObject.cpp:372
    frame #26: JSC::ProxyObject::getOwnPropertySlot(object=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbbd40, slot=0x00007ffeefbbc720) at ProxyObject.cpp:395
    frame #27: JSC::JSObject::getNonIndexPropertySlot(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbbea0, slot=0x00007ffeefbbc720) at JSObjectInlines.h:150
    frame #28: bool JSC::JSObject::getPropertySlot<false>(this=0x000062d000200e40, exec=0x000062d00002c048, propertyName=PropertyName @ 0x00007ffeefbbc320, slot=0x00007ffeefbbc720) at JSObject.h:1424
    frame #29: JSC::JSObject::calculatedClassName(object=0x000062d000200e40) at JSObject.cpp:535
    frame #30: JSC::Structure::toStructureShape(this=0x000062d000007410, value=JSValue @ 0x00007ffeefbbcae0, sawPolyProtoStructure=0x00007ffeefbbcf60) at Structure.cpp:1142
    frame #31: JSC::TypeProfilerLog::processLogEntries(this=0x000060400000a950, reason=0x00007ffeefbbd5c0) at TypeProfilerLog.cpp:89
    frame #32: JSC::JIT::doMainThreadPreparationBeforeCompile(this=0x0000619000034da0) at JIT.cpp:951
    frame #33: JSC::JITWorklist::Plan::Plan(this=0x0000619000034d80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:43
    frame #34: JSC::JITWorklist::Plan::Plan(this=0x0000619000034d80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:42
    frame #35: JSC::JITWorklist::compileLater(this=0x0000616000001b80, codeBlock=0x000062d0001d88c0, loopOSREntryBytecodeOffset=0) at JITWorklist.cpp:256
    frame #36: JSC::LLInt::jitCompileAndSetHeuristics(codeBlock=0x000062d0001d88c0, exec=0x00007ffeefbbde30, loopOSREntryBytecodeOffset=0) at LLIntSlowPaths.cpp:391
    frame #37: llint_replace(exec=0x00007ffeefbbde30, pc=0x00006040000161ba) at LLIntSlowPaths.cpp:516
    frame #38: llint_entry at LowLevelInterpreter64.asm:98
    frame #39: vmEntryToJavaScript at LowLevelInterpreter64.asm:296
    ...

This crash occurred because StackVisitor was seeing an invalid topCallFrame while
trying to capture the Error stack while throwing a StackOverflowError below
llint_replace.  While in this specific example, it is questionable whether we
should be executing JS code below TypeProfilerLog::processLogEntries(), it is
correct to have set the topCallFrame in llint_replace.  We do this by calling
LLINT_BEGIN_NO_SET_PC() at the top of llint_replace.

We also do the same for llint_osr.
        
Note: both of these LLInt slow path functions are called with a fully initialized
CallFrame.  Hence, there's no issue with setting topCallFrame to their CallFrames
for these functions.

* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@238141 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request May 4, 2019
https://bugs.webkit.org/show_bug.cgi?id=196025

Reviewed by Simon Fraser.

SVG attributes like "fill" and "stroke" do not have reflecting properties
in SVGElement but they are animatable by SMIL. Animating such attributes
is different from animating the SVG animated properties. These new classes
will be added to handle the first type of this kind of attribute: the Color:

-- SVGPropertyAnimatorCreator is added to SVGElement. It is responsible 
   for creating SVGPropertyAnimators for the attribute which do not have
   reflecting animated properties stored by SVGElement. It will maintain
   a HashMap for the animated values for these attributes which is indexed
   by the attribute name. The animated values has to be RefCounted because
   the same attribute can be animated by multiple animators. So the values
   of this HashMap will be of type Ref<SVGProperty>, e.g.
    <circle cx="80" cy="120" r="35">
        <animate attributeName="fill" values="#080" begin="2s" />
        <animate attributeName="fill" values="#602;#4" begin="4s" dur="5s"/>
    </circle>

-- SVGPropertyAnimator is the a new type which animates an attribute with
   no reflecting animated property.

-- SVGPrimitivePropertyAnimator is a template class which is responsible
   for animating attributes with primitive types, e.g. Color, string and 
   float. It is derived form SVGPropertyAnimator and it is initialized 
   with a Ref<SVGValueProperty<PropertyType>> which is created and maintained
   by SVGPropertyAnimatorFactory.

-- SVGAnimationColorFunction is the animation function that animates the
   attributes whose type are Color. Note the conversion form String to 
   Color in this class has to handle the case when its value is "attributeName="
   e.g. <animate attributeName="fill" from="attributeName="r"/>

-- SVGColorAnimator will be defined to be
   SVGPrimitivePropertyAnimator<Color, SVGAnimationColorFunction>.

The life cycle of the RefCounted properties can be explained as follows:

-- SVGPropertyAnimatorFactory checks whether its HashMap has an entry
   for the given attribute name. If it does not have, it will create a
   new value through the value creation method for this attribute.

-- SVGPropertyAnimatorFactory passes the shared animated value to the
   animator creation method. So multiple animators will be accessing the
   same value through their RefCounted pointers.

-- When the animator is about to be deleted, it will notify the target
   SVGElement which will notify its SVGPropertyAnimatorFactory.
   SVGPropertyAnimatorFactory will check its HashMap and retrieves the
   entry for the given attribute name. If the refCount is 2, it is going
   to remove the entry form the HashMap.

* Sources.txt:
* WebCore.xcodeproj/project.pbxproj:
* svg/SVGAnimateColorElement.cpp:
(WebCore::attributeValueIsCurrentColor): Deleted.
(WebCore::SVGAnimateColorElement::determinePropertyValueTypes): Deleted.
* svg/SVGAnimateColorElement.h:
* svg/SVGAnimateElementBase.cpp:
(WebCore::SVGAnimateElementBase::attributeAnimationController):
* svg/SVGAnimatedColor.cpp: Removed.
* svg/SVGAnimatedColor.h: Removed.
* svg/SVGAnimatorFactory.h:
(WebCore::SVGAnimatorFactory::create):
* svg/SVGAttributeAnimationController.cpp:
(WebCore::SVGAttributeAnimationController::~SVGAttributeAnimationController):
* svg/SVGAttributeAnimationController.h:
* svg/SVGElement.cpp:
(WebCore::SVGElement::SVGElement):
(WebCore::SVGElement::isAnimatedAttribute const):
(WebCore::SVGElement::createAnimator):
(WebCore::SVGElement::animatorWillBeDeleted):
* svg/SVGElement.h:
(WebCore::SVGElement::propertyAnimatorFactory):
* svg/SVGFitToViewBox.h:
* svg/SVGMPathElement.cpp:
* svg/graphics/filters/SVGFEImage.h:
* svg/properties/SVGAnimationAdditiveValueFunctionImpl.cpp: Added.
(WebCore::SVGAnimationColorFunction::colorFromString):
* svg/properties/SVGAnimationAdditiveValueFunctionImpl.h:
(WebCore::SVGAnimationColorFunction::progress):
* svg/properties/SVGAnimationFunction.h:
* svg/properties/SVGAttributeAnimator.cpp:
(WebCore::SVGAttributeAnimator::applyAnimatedStylePropertyChange):
(WebCore::SVGAttributeAnimator::removeAnimatedStyleProperty):
* svg/properties/SVGAttributeAnimator.h:
* svg/properties/SVGPrimitivePropertyAnimator.h: Added.
(WebCore::SVGPrimitivePropertyAnimator::create):
(WebCore::SVGPrimitivePropertyAnimator::SVGPrimitivePropertyAnimator):
* svg/properties/SVGPrimitivePropertyAnimatorImpl.h: Added.
* svg/properties/SVGPropertyAnimator.h: Added.
(WebCore::SVGPropertyAnimator::SVGPropertyAnimator):
(WebCore::SVGPropertyAnimator::adjustForInheritance const):
(WebCore::SVGPropertyAnimator::computeCSSPropertyValue const):
(WebCore::SVGPropertyAnimator::computeInheritedCSSPropertyValue const):
* svg/properties/SVGPropertyAnimatorFactory.h: Added.
(WebCore::SVGPropertyAnimatorFactory::isKnownAttribute):
(WebCore::SVGPropertyAnimatorFactory::createAnimator):
(WebCore::SVGPropertyAnimatorFactory::animatorWillBeDeleted):
(WebCore::SVGPropertyAnimatorFactory::createColorAnimator):
(WebCore::SVGPropertyAnimatorFactory::attributeAnimatorCreator):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@243259 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Jan 12, 2020
…wind and lldb.

https://bugs.webkit.org/show_bug.cgi?id=205050

Reviewed by Michael Saboff.

Before this patch, the stack trace from inside a probe function is cut off at ctiMasmProbeTrampoline:

    (lldb) bt
    * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
        ...
        frame #4: 0x0000000100824607 JavaScriptCore`WTF::Function<void (JSC::Probe::Context&)>::operator(this=0x000000010b88bd00, in=0x00007ffeefbfd400)(JSC::Probe::Context&) const at Function.h:79:35
        frame #5: 0x0000000100823996 JavaScriptCore`JSC::stdFunctionCallback(context=0x00007ffeefbfd400) at MacroAssembler.cpp:53:5
        frame #6: 0x000000010082701e JavaScriptCore`JSC::Probe::executeProbe(state=0x00007ffeefbfd480) at ProbeContext.cpp:51:5
        frame #7: 0x000000010082614b JavaScriptCore`ctiMasmProbeTrampoline + 299
    (lldb) 

After this patch, we'll now get the full stack trace from inside the probe function:

    (lldb) bt
    * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
        ...
        frame #4: 0x0000000100826d17 JavaScriptCore`WTF::Function<void (JSC::Probe::Context&)>::operator(this=0x0000000106b878f8, in=0x00007ffeefbfd400)(JSC::Probe::Context&) const at Function.h:79:35
        frame #5: 0x0000000100826106 JavaScriptCore`JSC::stdFunctionCallback(context=0x00007ffeefbfd400) at MacroAssembler.cpp:53:5
        frame #6: 0x000000010082986e JavaScriptCore`JSC::Probe::executeProbe(state=0x00007ffeefbfd480) at ProbeContext.cpp:51:5
        frame #7: 0x00000001008289a2 JavaScriptCore`ctiMasmProbeTrampoline + 338
        frame #8: 0x0000466db28025be
        frame #9: 0x0000000100754ffc JavaScriptCore`llint_entry at LowLevelInterpreter.asm:994
        frame #10: 0x0000000100738173 JavaScriptCore`vmEntryToJavaScript at LowLevelInterpreter64.asm:307
        frame #11: 0x0000000101489307 JavaScriptCore`JSC::JITCode::execute(this=0x0000000106ba1520, vm=0x0000000106d00000, protoCallFrame=0x00007ffeefbfd9b8) at JITCodeInlines.h:38:38
        frame #12: 0x0000000101488982 JavaScriptCore`JSC::Interpreter::executeProgram(this=0x0000000106bfd1f8, source=0x00007ffeefbff090, (null)=0x000000010d0e0000, thisObj=0x000000010d0e8020) at Interpreter.cpp:847:51
        frame #13: 0x00000001017d1f9c JavaScriptCore`JSC::evaluate(globalObject=0x000000010d0e0000, source=0x00007ffeefbff090, thisValue=JSValue @ 0x00007ffeefbfef60, returnedException=0x00007ffeefbff0b0) at Completion.cpp:146:38
        frame #14: 0x000000010005838f jsc`runWithOptions(globalObject=0x000000010d0e0000, options=0x00007ffeefbff620, success=0x00007ffeefbff48b) at jsc.cpp:2670:35
        frame #15: 0x000000010002a0da jsc`jscmain(this=0x00007ffeefbff5a0, vm=0x0000000106d00000, globalObject=0x000000010d0e0000, success=0x00007ffeefbff48b)::$_6::operator()(JSC::VM&, GlobalObject*, bool&) const at jsc.cpp:3157:13
        frame #16: 0x0000000100006eff jsc`int runJSC<jscmain(int, char**)::$_6>(options=0x00007ffeefbff620, isWorker=false, func=0x00007ffeefbff5a0)::$_6 const&) at jsc.cpp:3003:9
        frame #17: 0x0000000100005988 jsc`jscmain(argc=10, argv=0x00007ffeefbff6c8) at jsc.cpp:3150:18
        frame #18: 0x000000010000575e jsc`main(argc=10, argv=0x00007ffeefbff6c8) at jsc.cpp:2498:15
        frame #19: 0x00007fff6cfc4da9 libdyld.dylib`start + 1
        frame #20: 0x00007fff6cfc4da9 libdyld.dylib`start + 1
    (lldb)

The difference is that the x86_64 ctiMasmProbeTrampoline now uses the standard
function prologue, and keeps %rbp pointing to trampoline function's semblance of
a frame that libunwind can understand while it calls the probe function.

* assembler/MacroAssemblerX86Common.cpp:



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253320 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Feb 6, 2020
…rchability.

https://bugs.webkit.org/show_bug.cgi?id=207024

Reviewed by Saam Barati.

This patch applies the following changes:

1. Prefix Air and B2 dumps with a tierName prefix.
   The tierName prefix strings are as follows:

       "FTL ", "DFG ", "b3  ", "Air ", "asm "

   The choice to use a lowercase "b3" and "asm" with upper case "Air" is
   deliberate because I found this combination to be easier to read and scan as
   prefixes of the dump lines.  See dump samples below.

2. Make DFG node IDs consistently expressed as D@<node index> e.g. D@104.
   The definition of the node will be the id followed by a colon e.g. D@104:
   This makes it easy to search references to this node anywhere in the dump.

   Make B3 nodes expressed as b@<node index> e.g. b@542.
   This also makes it searchable since there's now no ambiguity between b@542 and
   D@542.

   The choice to use a lowercase "b" and an uppercase "D" is intentional because
   "b@542" and "d@542" looks too similar, and I prefer to not use too much
   uppercase.  Plus this makes the node consistent in capitalization with the
   tierName prefixes above of "b3  " and "DFG " respectively.

Here's a sample of what the dumps now look like:

DFG graph dump:
<code>
    ...
         6 55:   <-- foo#DFndCW:<0x62d0000b8140, bc#65, Call, known callee: Object: 0x62d000035920 with butterfly 0x0 (Structure %AN:Function), StructureID: 12711, numArgs+this = 1, numFixup = 0, stackOffset = -16 (loc0 maps to loc16)>
      3  6 55:   D@79:< 3:->    ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid)
      4  6 55:    D@3:<!0:->    KillStack(MustGen, loc7, W:Stack(loc7), ClobbersExit, bc#71, ExitInvalid)
      5  6 55:   D@85:<!0:->    MovHint(Check:Untyped:D@79, MustGen, loc7, W:SideState, ClobbersExit, bc#71, ExitInvalid)
      6  6 55:  D@102:< 1:->    CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid)
      7  6 55:  D@104:<!0:->    Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:#7/w:1.000000, W:SideState, bc#74, ExitInvalid)
    ...
</code>

B3 graph dump:
<code>
    ...
    b3  BB#14: ; frequency = 10.000000
    b3    Predecessors: #13
    b3      Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [], ExitsSideways|Reads:Top, D@79)
    b3      Int32 b@539 = LessThan(b@531, $100(b@578), D@102)
    b3      Void b@542 = Branch(b@539, Terminal, D@104)
    b3    Successors: Then:#2, Else:#15
    ...
</code>

Air graph dump:
<code>
    ...
    Air BB#5: ; frequency = 10.000000
    Air   Predecessors: #4
    Air     Move -96(%rbp), %rax, b@531
    Air     Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531
    Air     Branch32 LessThan, %rax, $100, b@542
    Air   Successors: #1, #6
    ...
</code>

FTL disassembly dump:
<code>
    ...
    Air BB#5: ; frequency = 10.000000
    Air   Predecessors: #4
    DFG       D@42:< 2:->   JSConstant(JS|PureInt, Int32, Int32: 1, bc#0, ExitInvalid)
    DFG       D@79:< 3:->   ArithAdd(Int32:Kill:D@95, Int32:D@42, Int32|PureNum|UseAsOther, Int32, CheckOverflow, Exits, bc#71, ExitValid)
    b3            Int32 b@1 = Const32(1)
    b3            Int32 b@531 = CheckAdd(b@10:WarmAny, $1(b@1):WarmAny, b@64:ColdAny, b@10:ColdAny, generator = 0x606000022e80, earlyClobbered = [], lateClobbered = [], usedRegisters = [%rax, %rbx, %rbp, %r12], ExitsSideways|Reads:Top, D@79)
    Air               Move -96(%rbp), %rax, b@531
    asm                   0x4576b9c04712: mov -0x60(%rbp), %rax
    Air               Patch &BranchAdd32(3,ForceLateUseUnlessRecoverable)3, Overflow, $1, %rax, -104(%rbp), -96(%rbp), b@531
    asm                   0x4576b9c04716: inc %eax
    asm                   0x4576b9c04718: jo 0x4576b9c04861
    DFG       D@89:< 1:->   JSConstant(JS|PureNum|UseAsOther, NonBoolInt32, Int32: 100, bc#0, ExitInvalid)
    DFG      D@102:< 1:->   CompareLess(Int32:D@79, Int32:D@89, Boolean|UseAsOther, Bool, Exits, bc#74, ExitValid)
    DFG      D@104:<!0:->   Branch(KnownBoolean:Kill:D@102, MustGen, T:#1/w:10.000000, F:#7/w:1.000000, W:SideState, bc#74, ExitInvalid)
    b3            Int32 b@578 = Const32(100, D@89)
    b3            Int32 b@539 = LessThan(b@531, $100(b@578), D@102)
    b3            Void b@542 = Branch(b@539, Terminal, D@104)
    Air               Branch32 LessThan, %rax, $100, b@542
    asm                   0x4576b9c0471e: cmp $0x64, %eax
    asm                   0x4576b9c04721: jl 0x4576b9c0462f
    Air   Successors: #1, #6
    ...
</code>

* b3/B3BasicBlock.cpp:
(JSC::B3::BasicBlock::deepDump const):
* b3/B3Common.cpp:
* b3/B3Common.h:
* b3/B3Generate.cpp:
(JSC::B3::generateToAir):
* b3/B3Procedure.cpp:
(JSC::B3::Procedure::dump const):
* b3/B3Value.cpp:
* b3/air/AirBasicBlock.cpp:
(JSC::B3::Air::BasicBlock::deepDump const):
(JSC::B3::Air::BasicBlock::dumpHeader const):
(JSC::B3::Air::BasicBlock::dumpFooter const):
* b3/air/AirCode.cpp:
(JSC::B3::Air::Code::dump const):
* b3/air/AirCode.h:
* b3/air/AirDisassembler.cpp:
(JSC::B3::Air::Disassembler::dump):
* b3/air/AirGenerate.cpp:
(JSC::B3::Air::prepareForGeneration):
* dfg/DFGCommon.cpp:
* dfg/DFGCommon.h:
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
(JSC::DFG::Graph::dumpBlockHeader):
* dfg/DFGNode.cpp:
(WTF::printInternal):
* ftl/FTLCompile.cpp:
(JSC::FTL::compile):
* ftl/FTLCompile.h:
* ftl/FTLState.cpp:
(JSC::FTL::State::State):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@255482 268f45cc-cd09-0410-ab3c-d52691b4dbfc
pulkomandy pushed a commit that referenced this pull request Aug 10, 2020
https://bugs.webkit.org/show_bug.cgi?id=213436

Reviewed by Antti Koivisto.

Source/WebCore:

1. The table generates a principal block container box called the table wrapper box that contains the table box itself and any caption boxes.
2. The table wrapper box establishes a block formatting context, and the table box establishes a table formatting context.
3. The computed values of properties 'position', 'float', 'margin-*', 'top', 'right', 'bottom', and 'left' on
   the table element are used on the table wrapper box and not the table box; all other values of non-inheritable
   properties are used on the table box and not the table wrapper box.
4. In a block formatting context, each box's left outer edge touches the left edge of the containing block.
   This is true even in the presence of floats, unless the box establishes a new block formatting context (in which case the
   box itself may become narrower due to the floats)

Now consider the following case:
<div style="display: block; width: 500px;">
    <div style="float: left; width: 100px;"></div>
    <div style="display: table; width: 10%;"></div>
</div>
1. We create a table wrapper box to wrap the "display: table" block level box (#1).
2. The table wrapper box's width property is set to auto (#3).
3. Since it establishes a new block formatting context, the available horizontal space gets shrunk by the float (#4)
4. The table wrapper box's used width computes to 500px - 100px -> 400px;

Now we are inside the BFC established by the table wrapper box and try to resolve the table's width -> %10.
According to the normal BFC rules, it should compute to 10% of the containing block's logical width: 400px -> 40px.
However in practice it computes to 50px (10% of 500px).

Similar setup with non-table content would resolve the inner block level box's width to 40px;
<div style="display: block; width: 500px">
    <div style="float: left; width: 100px;"></div>
    <div style="display: block; overflow: hidden;">
        <div style="display: block; width: 10%"></div>
    </div>
</div>
This needs clarification.

Test: fast/layoutformattingcontext/float-avoider-available-horizontal-space3.html

* layout/FormattingContext.h:
(WebCore::Layout::FormattingContext::isTableWrapperBlockFormattingContext const):
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::layoutInFlowContent):
* layout/blockformatting/tablewrapper/TableWrapperBlockFormattingContext.cpp:
(WebCore::Layout::TableWrapperBlockFormattingContext::computeWidthAndMarginForTableBox):
* layout/blockformatting/tablewrapper/TableWrapperBlockFormattingContext.h:

LayoutTests:

* fast/layoutformattingcontext/float-avoider-available-horizontal-space3-expected.html: Added.
* fast/layoutformattingcontext/float-avoider-available-horizontal-space3.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@263327 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants