-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fixed missing headers from Webcore for Webkit2 #8
Conversation
along with this library; see the file COPYING.LIB. If not, write to | ||
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, | ||
Boston, MA 02110-1301, USA. | ||
* Copyright (C) 2016 Haiku, Inc. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about this file, where does it come from? Why is the copyright year 2016?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am sorry @pulkomandy i was trying to fix workqueue initially it had not implemented functions. But when i saw about workqueue i found it should be under web template framework as it is the one which handles threads and stuff. so got confused little bit 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will restore this part right now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, this was moved from WebKit to WTF. So this file can be deleted, then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i guess workqueue is handled by wtf so no need in webkit right?
Source/WebKit/PlatformHaiku.cmake
Outdated
@@ -131,3 +131,14 @@ add_definitions( | |||
-DNETWORKPROCESSNAME=\"NetworkProcess\" | |||
) | |||
|
|||
set(WebKit_FORWARDING_HEADERS_DIRECTORIES | |||
shared/API/c | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't leave empty lines here
@@ -41,7 +41,8 @@ | |||
#endif | |||
|
|||
#if PLATFORM(HAIKU) | |||
#include "graphics/haiku/StillImageHaiku.h" | |||
//#include "graphics/haiku/StillImageHaiku.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this commented line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The work queue implementation seems to be sound.
@@ -26,6 +26,7 @@ | |||
|
|||
#pragma once | |||
|
|||
#include "AffineTransform.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually I try to get this working without modifying the WebKit and WebCore headers.
My solution is to move WebKit and webcore includes to local includes in the CMakeLists (you can see how this is done in PlatformHaiku.cmake for WebKitLegacy).
This way, the include with quotes will include WebKit header, and the include with brackets will include Haiku headers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy ur saying about local include directories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy i did this because to avoid the forward declaration problem that was stopping the build!
If i understood correctly affinetransform.h is referring to haiku header? but i could only find a common header 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case it is referring to the WebCore header, but if you do nothing special, #include "AffineTransform.h" will include the Haiku header instead, and the build will be confused.
@@ -71,6 +72,10 @@ class AffineTransform { | |||
AffineTransform(const D2D1_MATRIX_3X2_F&); | |||
#endif | |||
|
|||
#if PLATFORM(HAIKU) | |||
AffineTransform(const BAffineTransform&); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this was not here, or at least marked explicit to avoid accidentally converting between the two (it could be a performance problem). There are already functions to convert between the two types where needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy so including webkit headers would have BAffineTransform within?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if you do this:
#include "AffineTransform.h" // get the WebCore header
#include <AffineTransform.h> // get the Haiku header
There are several other cases of similar header clashes in the codebase, which is the reason for introducing LOCAL_INCLUDE_DIRECTORIES in the CMakeLists. The same change can be applied in the CMakeLists for WebKit(2)
Source/WTF/wtf/CurrentTime.h
Outdated
@@ -0,0 +1,86 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this new file needed? Wasn't it moved elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this file is under performance tests but building required this file . So i moved it here.
🤔
Hi, I have rebased your changes and pushed them to the webkit2 branch, with some fixes. I stopped at an issue I don't know how to solve, so your turn to investigate! The changes I made should give you an idea how various problems can be fixed. I think the CMakeLists.txt in WebKit/ is out of date, so maybe compare it with the current version in github.com/webkit/webkit and decide which changes are relevant, and which should be dropped. This also applies potentially to all files in the WebKit directory, if in doubt, make sure they are in sync with the versions in the upstream webkit repository. I have included some extra notes about what I did in the commit message. Don't hesitate to rework your commits (using git rebase -i for example), so that the overall history makes sense. It is easier to follow if a commits does not reverse what was done in the previous one :) |
@@ -31,6 +31,8 @@ | |||
|
|||
#if defined(WIN32) || defined(_WIN32) | |||
typedef int WKProcessID; | |||
#elif PLATFORM(HAIKU) | |||
typedef int32_t WKProcessID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not right, pid_t should be used for WKProcessID. Adjust the places where it is encoded/decoded if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as i know WTF::ProcessID should be same as WKProcessID . We gave WTF::ProcessID as int32_t explicitly so there is ambuiguity rised in function definitions because WKProcessID is of pid_t.
So u think setting WKPRocessID to int32_t explicitly like we did for WTF::processid is viable solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to keep pid_t and use the toPid/toProcessId functions I added to convert between the two types?
But if that does not work, yes, we will need this change. In this case I wonder why the code does not use WTF::ProcessID here and defines a different type...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering the same 😕 . We need not use converting right as having int32_t is simple and viable solution will not introduce much merge conflicts what say?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case we have found a real problem in WebKit code, and we will send them the change. So we can make more modifications, and once they have accepted the patch, there will be no merge conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So can i right away explicitly convert to int32_t the WKProcessID?
I asked the webkit-dev still now there was no response so when they suggest something better we better adapt that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They already replied to my mail previously and you asked the same question again. Surely they will find this boring and will not reply a second time.
As I said, I added the toPid/toProcessId functions so we can use them to convert between WKProcessID (pid_t) and WTF::ProcessID (int32_t) and back
@@ -31,6 +31,8 @@ | |||
|
|||
#if defined(WIN32) || defined(_WIN32) | |||
typedef int WKProcessID; | |||
#elif PLATFORM(HAIKU) | |||
typedef int32_t WKProcessID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here (and why is this enum repeated anyways?)
@@ -2077,7 +2077,7 @@ WebsiteDataStoreParameters WebsiteDataStore::parameters() | |||
SandboxExtension::createHandleForReadWriteDirectory(parameters.serviceWorkerRegistrationDirectory, parameters.serviceWorkerRegistrationDirectoryExtensionHandle); | |||
#endif | |||
|
|||
platformSetNetworkParameters(parameters); | |||
//platformSetNetworkParameters(parameters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not edit generic WebKit headers, unless they are out of sync with upstream webkit code because of an earlier merge problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy i was trying to debug thats why commented it . iforgot to revert it back 😕 sorry.
Source/WTF/wtf/ProcessID.h
Outdated
@@ -36,7 +36,9 @@ | |||
namespace WTF { | |||
|
|||
#if OS(WINDOWS) | |||
using ProcessID = int; | |||
using ProcessID = int; | |||
#elif PLATFORM(HAIKU) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy this is also not allowed? should i use pid_t and cast it to int32_t?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy alright it is also un conventional so let me fix by casting itself 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pulkomandy am i allowed to add casting in generic file if so where can i add?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we are forced to change generic files anyways. (ProcessID.h is also a generic file :))
So, let's add the cast, or maybe add specific calls to the decoders/encoders to encode/decode a pid_t. I don't know which solution is preferred, I will ask webkit-dev.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have pushed a work in progress implementation which does what I think the WebKit developers want (but I'm not sure). I did not get the "decode" part working yet. I think you can work from that (unless they changed their mind while I was sleeping).
Hi, Today I cleaned up your commits to not have one undoing another, removed some changes I think are incorrect (to AffineTransform, and introduction of CurrentTime.h in wtf which doesn't exist in upstream webkitm for example). I have changed the ProcessID implementation again, I think this is what Sam Weining meant on the webkit mailing list, however the conversion macros are not used because implicit conversion will work. Currently the build fails with: ../../Source/WebKit/NetworkProcess/CustomProtocols/LegacyCustomProtocolManager.h:92:32: error: 'CustomProtocol' has not been declared |
@@ -0,0 +1,21 @@ | |||
/* | |||
license |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a standard license for webkit, please use it in all files you create:
https://trac.webkit.org/export/242591/webkit/trunk/Source/WebCore/LICENSE-APPLE
of course replace copyright years and assign copyright either to yourself or to Haiku, inc, not to Apple.
#include <pal/crypto/gcrypt/Initialization.h> | ||
#endif | ||
#endif*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the error i told that day the compiler couldnt identify USE maybe its because the source is happening in a seperate process?
@@ -227,6 +230,12 @@ bool WebFrameLoaderClient::shouldUseCredentialStorage(DocumentLoader*, unsigned | |||
return webPage->injectedBundleResourceLoadClient().shouldUseCredentialStorage(*webPage, *m_frame, identifier); | |||
} | |||
|
|||
bool WebFrameLoaderClient::dispatchDidReceiveInvalidCertificate(WebCore::DocumentLoader*, const WebCore::CertificateInfo&, const char* message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should i also add if directive to check if its haiku?
notImplemented(); | ||
return nullptr; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted it accidentally and moved it in back but its showing deleted 😞 .
{ | ||
notImplemented(); | ||
return false; | ||
} | ||
|
||
void ArgumentCoder<ResourceResponse>::encodePlatformData(ArgumentEncoder& encoder, const ResourceResponse& resourceResponse) | ||
/*void ArgumentCoder<ResourceResponse>::encodePlatformData(Encoder& encoder, const ResourceResponse& resourceResponse) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one doesnt have a matching template in webcoderarguement. so shall i add a template for resourceresponse ? i suppose other platforms dont use resourceresponse so i commented it .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a resourceError template i think it covers response? what say
{ | ||
//encoder << static_cast<uint32_t>(resourceResponse.soupMessageFlags()); | ||
} | ||
|
||
bool ArgumentCoder<ResourceResponse>::decodePlatformData(ArgumentDecoder& decoder, ResourceResponse& resourceResponse) | ||
bool ArgumentCoder<ResourceResponse>::decodePlatformData(Decoder& decoder, ResourceResponse& resourceResponse) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
static NeverDestroyed<NetworkProcess> networkProcess(WTFMove(parameters)); | ||
} | ||
|
||
int NetworkProcessMainUnix(int argc, char** argv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use the existing NetworkProcessMainUnix? When the UNIX code works; there is no need to copy it, and copying it and changing copyright on it is not right (you should keep the original copyright when you make changes and add the haiku copyright as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only declaration is provided by Unix we have to define it for our own platform hence this
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2014 Haiku, inc. | |||
* Copyright 2019 Haiku, Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2014-2019
@@ -32,8 +32,8 @@ | |||
#include "MIMETypeRegistry.h" | |||
#include "NotImplemented.h" | |||
#include "StillImageHaiku.h" | |||
#include "JavaScriptCore/GenericTypedArrayViewInlines.h" | |||
#include "JavaScriptCore/JSGenericTypedArrayView.h" | |||
#include "JavaScriptCore/JSCInlines.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked that the changes in WebCore do not build the WebKitLegacy build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing webcore doesnt build webkitlegacy? i thought all the linked files would be rebuilt again not sure let me build again and let you know 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can build DumpRenderTree or HaikuLauncher (ninja HaikuLauncher) to test building just the webkitlegacy parts. Since WebKit2 build is broken the build may stop before or after building webkit...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is absolutely safe Pulkomandy 😄
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2013 Intel Corporation. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous copyright should be kept if the file was initially a copy of the work done for another platform.
…string Note:destructors not handled yet
…of posting a message to main run loop
@@ -100,7 +100,7 @@ void RunLoop::run() | |||
looper->LockLooper(); | |||
looper->AddHandler(current().m_handler); | |||
looper->UnlockLooper(); | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will get the spacing and weird debug messages out as now my only aim is to get it rendering 😄
@@ -115,7 +115,8 @@ void RunLoop::stop() | |||
|
|||
void RunLoop::wakeUp() | |||
{ | |||
m_handler->Looper()->PostMessage('loop', m_handler); | |||
RunLoop::current().performWork(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, this look completely wrong!
RunLoop::wakeUp is called from other threads to wake up the loop.
performWork should be run inside the main loop thread.
There is nothing to win in taking such shortcuts. You will only get things to crash in very strange ways.
Also, this probably breaks existing use of runloop in WebKitLegacy. It would be great if I could merge your changes in the main branch, but with changes like this, it's not going to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry just was curious what happens beyond this point 😢 but i knew it is going to break things i will find howto make it work.
@@ -96,7 +96,7 @@ list(APPEND WebCore_SOURCES | |||
platform/graphics/texmap/GraphicsLayerTextureMapper.cpp | |||
platform/graphics/texmap/TextureMapperImageBuffer.cpp | |||
|
|||
platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp | |||
#platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the problem with this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file does not do anything everything is wrapped under USE(COORDINATEDGRAPHICS) while we have disabled it so thought it would be a good clean up 😅😅
@@ -117,7 +117,7 @@ WTF::Optional<size_t> BFormDataIO::readFromFile(const FormDataElement::EncodedFi | |||
m_fileHandle = FileSystem::openFile(fileData.filename, FileSystem::FileOpenMode::Read); | |||
|
|||
if (!FileSystem::isHandleValid(m_fileHandle)) { | |||
LOG(Network, "Haiku - Failed while trying to open %s for upload\n", fileData.filename.utf8().data()); | |||
//LOG(Network, "Haiku - Failed while trying to open %s for upload\n", fileData.filename.utf8().data()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Is it just a missing include to define the LOG macro?
{ | ||
fprintf(stderr,"its painting\n"); | ||
WebCore::Region unpainted; | ||
BView* surface = new BView("drawing_surface",B_WILL_DRAW); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BWebView is a BView, and it's the one you should draw on. Here you are creating a new BView every time this is called, which is a huge memory leak, and you are not going to draw anything onto the BWebView itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops im still learning so please forgive me. I do understand that it is wrong now. Uh how can i miss it out I am supposed to use the existing view to draw but im just overlaying it over and over.
@@ -26,12 +26,83 @@ | |||
#include "config.h" | |||
#include "ProcessLauncher.h" | |||
|
|||
#define __STDC_FORMAT_MACROS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes to get uint64 into a string(as a format specifier) we need this
@@ -0,0 +1,16 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its like a configuration file created by webkit for mini browser
@@ -39,13 +39,15 @@ class WebProcessMain final : public AuxiliaryProcessMainBase { | |||
public: | |||
bool platformInitialize() override | |||
{ | |||
BApplication* app = new BApplication("application/x-vnd.haiku-webkit.webprocess"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You really need a BApplication to do anything. Where is it created, if not here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ui process is a bapplication right wont that suffice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Without the BApplication, 1) the worker will have no link to the app_server for things like drawing ops, and 2) It won't be registered with the roster, and consequently you'd have no way of sending e.g. BMessages between it and the UI process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see so that explains why my messages were not recieved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this will be my main run loop that gets all my messages right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For inter-process communications, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I think the runloop implementation detects that it is in the main thread, and in that cases it will run the BApplication looper (using be_app->Run()). Otherwise (for various worker threads) it will create its own worker instead.
…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
Fixed some of the missing headers for the day
Next target is Webkit itself