Bug 225551 – [GLIB] REGRESSION(r277158) imported/w3c/web-platform-tests/xhr/FormData-append.html is crashing

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Diego Pino]

The test started crashing after r277158:

https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fxhr%2FFormData-append.html&platform=GTK&platform=WPE&platform=ios&platform=mac

Stack trace:

https://build.webkit.org/results/GTK-Linux-64-bit-Debug-Tests/r277196%20(1013)/imported/w3c/web-platform-tests/xhr/FormData-append-crash-log.txt

Thread 1 (Thread 0x7f29685acec0 (LWP 6316)):
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
#1  0x00007f29724d311c in std::char_traits<char>::length(char const*) (__s=0x0) at /usr/include/c++/10.2.0/bits/char_traits.h:357
#2  0x00007f2973895546 in std::basic_string_view<char, std::char_traits<char> >::basic_string_view(char const*) (this=0x7ffc1ed0ede0, __str=0x0) at /usr/include/c++/10.2.0/string_view:128
#3  0x00007f296ed0cc5f in std::filesystem::__cxx11::path::_S_convert(char const*, std::filesystem::__cxx11::__detail::__null_terminated) (__src=0x0) at /usr/include/c++/10.2.0/bits/fs_path.h:541
#4  0x00007f296ed0daa7 in std::filesystem::__cxx11::path::path<char const*, std::filesystem::__cxx11::path>(char const* const&, std::filesystem::__cxx11::path::format) (this=0x7ffc1ed0eec0, __source=@0x7ffc1ed0eea8: 0x0) at /usr/include/c++/10.2.0/bits/fs_path.h:225
#5  0x00007f296ed0b9f5 in WTF::FileSystemImpl::getFileModificationTime(WTF::String const&) (path=...) at ../../Source/WTF/wtf/FileSystem.cpp:667
#6  0x00007f29757cdc39 in WebCore::File::lastModified() const (this=0x7f29248ce3b0) at ../../Source/WebCore/fileapi/File.cpp:113
#7  0x00007f2973d39329 in WebCore::jsFile_lastModifiedGetter(JSC::JSGlobalObject&, WebCore::JSFile&) (lexicalGlobalObject=..., thisObject=...) at WebCore/DerivedSources/JSFile.cpp:285
#8  0x00007f2973d437e9 in WebCore::IDLAttribute<WebCore::JSFile>::get<WebCore::jsFile_lastModifiedGetter, (WebCore::CastedThisErrorBehavior)3>(JSC::JSGlobalObject&, JSC::EncodedJSValue, JSC::PropertyName) (lexicalGlobalObject=..., thisValue=139814311776008, attributeName=...) at ../../Source/WebCore/bindings/js/JSDOMAttribute.h:90
#9  0x00007f2973d3939f in WebCore::jsFile_lastModified(JSC::JSGlobalObject*, JSC::EncodedJSValue, JSC::PropertyName) (lexicalGlobalObject=0x7f290c2e0000, thisValue=139814311776008, attributeName=...) at WebCore/DerivedSources/JSFile.cpp:290
#10 0x00007f296e88685a in JSC::PropertySlot::customGetter(JSC::JSGlobalObject*, JSC::PropertyName) const (this=0x7ffc1ed0f360, globalObject=0x7f290c2e0000, propertyName=...) at ../../Source/JavaScriptCore/runtime/PropertySlot.cpp:46
#11 0x00007f296d248bd3 in JSC::PropertySlot::getValue(JSC::JSGlobalObject*, JSC::PropertyName) const (this=0x7ffc1ed0f360, globalObject=0x7f290c2e0000, propertyName=...) at ../../Source/JavaScriptCore/runtime/PropertySlot.h:408
#12 0x00007f296d2695f5 in JSC::JSValue::get(JSC::JSGlobalObject*, JSC::PropertyName, JSC::PropertySlot&) const (this=0x7ffc1ed0f2a0, globalObject=0x7f290c2e0000, propertyName=..., slot=...) at ../../Source/JavaScriptCore/runtime/JSCJSValueInlines.h:950
#13 0x00007f296e3af5db in JSC::LLInt::performLLIntGetByID(JSC::Instruction const*, JSC::CodeBlock*, JSC::JSGlobalObject*, JSC::JSValue, JSC::Identifier const&, JSC::GetByIdModeMetadata&) (pc=0x7f290c4f55d5, codeBlock=0x7f290d88e2e0, globalObject=0x7f290c2e0000, baseValue=..., ident=..., metadata=...) at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:760
#14 0x00007f296e3afdf9 in JSC::LLInt::llint_slow_path_get_by_id(JSC::CallFrame*, JSC::Instruction const*) (callFrame=0x7ffc1ed0f5b0, pc=0x7f290c4f55d5) at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:834
#15 0x00007f296d215359 in llint_op_get_by_id () at /app/webkit/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:97

[Diego Pino]

Created attachment 428068 [details]
Patch

[Chris Dumez]

Note that fileSystemRepresentation() seems wrong for the glib port. It returns CString() instead of CString("") when the string is the empty string. It should only return CString() is the String is null.

[Chris Dumez]

(In reply to Chris Dumez from comment #2)
> Note that fileSystemRepresentation() seems wrong for the glib port. It
> returns CString() instead of CString("") when the string is the empty
> string. It should only return CString() is the String is null.

Maybe fixing Glib's fileSystemRepresentation() would mean we wouldn't need those isEmpty() checks in all FileSystem functions, unless glib ports are really calling those functions will null Strings..

[Diego Pino]

Created attachment 428076 [details]
Patch

[Diego Pino]

(In reply to Chris Dumez from comment #3)
> (In reply to Chris Dumez from comment #2)
> > Note that fileSystemRepresentation() seems wrong for the glib port. It
> > returns CString() instead of CString("") when the string is the empty
> > string. It should only return CString() is the String is null.
> 
> Maybe fixing Glib's fileSystemRepresentation() would mean we wouldn't need
> those isEmpty() checks in all FileSystem functions, unless glib ports are
> really calling those functions will null Strings..

Thanks for the pointer. I also agree fixing this error at GLib is a better fix. Patch updated.

[Diego Pino]

Created attachment 428077 [details]
Patch

[Chris Dumez]

Comment on attachment 428077 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428077&action=review

> Source/WTF/wtf/glib/FileSystemGlib.cpp:77
> +        return CString("", 0);

I think this should be:
If (path.isNull())
  Return { };
If (path.isEmpty())
  Return CString(“”);

Seems weird to return an empty string if the input string was null.

[Chris Dumez]

Comment on attachment 428077 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428077&action=review

> Source/WTF/wtf/glib/FileSystemGlib.cpp:77
> +        return CString("", 0);

I think this should be:
If (path.isNull())
  Return { };
If (path.isEmpty())
  Return CString(“”);

Seems weird to return an empty string if the input string was null.

[Diego Pino]

Created attachment 428083 [details]
Patch

[EWS]

Committed r277229 (237498@main): <https://commits.webkit.org/237498@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428083 [details].

[Radar WebKit Bug Importer]

<rdar://problem/77697036>