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

C init #549

Merged
merged 3 commits into from
Jan 10, 2019
Merged

C init #549

merged 3 commits into from
Jan 10, 2019

Conversation

AJenbo
Copy link
Member

@AJenbo AJenbo commented Jan 4, 2019

This allows us to compile the following 6 files as C
dthread.cpp dx.cpp logging.cpp fault.cpp msgcmd.cpp nthread.cpp engine.cpp

See https://www.codeguru.com/cpp/misc/misc/applicationcontrol/article.php/c6945/Running-Code-Before-and-After-Main.htm for what's being done here.

Most importantly things still compiles and runs as C++ and under other compilers.

@AJenbo AJenbo mentioned this pull request Jan 4, 2019
38 tasks
@squidcc

This comment has been minimized.

@mewmew
Copy link
Contributor

mewmew commented Jan 6, 2019

Storm was obviously written in C++ and these startup functions must have used something from it's header to get linked correctly.

Interesting. Do you know what it may have looked like? It would be great if we could reconstruct this!

@AJenbo
Copy link
Member Author

AJenbo commented Jan 6, 2019

@squidcc When you start you comment with "Not sure where/why you...", it's easy to feel like you're criticizing the person making the PR and not the implementation. If you instead start with your findings and then follow with your recommended change it becomes a factual argument and much less likely to be taken the wrong way.

About this solution; while we talked on the chat we came to the believe that some sort of C++ helper was used, but we currently do not know how this would be implemented. The initial plan there for is to get things running so that we can get the compiler chain matching what is indicated by the rich header, then once that is done we would focus on how to get the different parts bin exact. When looking at using data_seg we felt that putting it in existing segments might not be safe/good practice. In the article I linked there are also some thoughts on this matter as well. XIM was one that I cam across a few times as a recommendation when I was doing further research on the subject, it simply puts it in the middle of the C init range (XIA-XIZ), and before globals are initialized (XIU).
Of-cause, as mewmew mentioned, if someone provides a solution that is likely to be correct from the start that is the preferred way to go instead of a temporary solution.

dthread.cpp dx.cpp init logging.cpp fault.cpp msgcmd.cpp nthread.cpp
@AJenbo
Copy link
Member Author

AJenbo commented Jan 6, 2019

PR rebased on nightly and engine.cpp is now also compiled as C

@squidcc

This comment has been minimized.

@sskras
Copy link
Contributor

sskras commented Jan 6, 2019

Quoting @AJenbo :

About this solution; while we talked on the chat we came to the believe that some sort of C++ helper was used, but we currently do not know how this would be implemented. The initial plan there for is to get things running so that we can get the compiler chain matching what is indicated by the rich header,

I have lost track of project's changes already so asking directly.

Could it be that structs *_c_init() / *_cpp_init_2() were initialized in a single Cpp-compiled module which I imagine could be one or two in the whole project? Wouldn't it simplify the init implementation? I remember a discussion about some flags discovered from headers (some time ago) but forgot the whole picture of it.

@AJenbo
Copy link
Member Author

AJenbo commented Jan 6, 2019

It means I would like to hear your justification since there appears to be no explanation in the description.

Ok, remember that a lot of this is happening on the chat so if you want to be deeply involved with the project and know the what and why, then that would be a good place to go. Is there something about Discord that doesn't work for you?

But unfortunately it's not correct/does not match the original binary (which uses XCU).

Do you think that this solution would get us close to bin exact if we changed it from XIM to XCU? I feel that this method is a bit hackish and probably won't be bin exact and so would like to do it a bit conservatively, hence using a unused section.
What tools do you use to analyze this part of the code?

You especially do not want these functions called before the CRT's C globals are initialized (XIU)

My understaning is that CRT's C globals are initialized during XIA, XIAA, XIC & XID, afaik XIU is only where own application variables gets initialized. I have also verified that the game works and that onexit functions are called correctly with this solution.

@AJenbo
Copy link
Member Author

AJenbo commented Jan 6, 2019

Could it be that structs *_c_init() / *_cpp_init_2() were initialized in a single Cpp-compiled module which I imagine could be one or two in the whole project?

It might be possible, but it wouldn't really give you any benefits over simply calling them from the start of main() (programming wise) so I don't think that was the case, the rich header indicates that no files where processed as C++, however it's possible that they used a precompiled helper compiled with an older version of VC (5 sp2 or older afaik) that didn't produce the needed information to get included in the rich header, but that would mean that they couldn't update it as the code evolved, so how exactly that would work we are unsure of.

I remember a discussion about some flags discovered from headers (some time ago) but forgot the whole picture of it.

You might be thinking of the INFINITY property, it appeared in any file that used critical sections, but was unused. There's there for a good chance that there was a helper libery used that took care of initialization the needed functionality (it's header would have defined INFINITY).

@squidcc

This comment has been minimized.

@AJenbo

This comment has been minimized.

@mewmew
Copy link
Contributor

mewmew commented Jan 6, 2019

@mewmew how do you feel about changing this to use XCU?

To be honest this is outside of my knowledge or expertise. However, if changing to XCU does not break the game, then sure go for it! Eventually, we want to try to nail the original implementation anyways, so this is more of a temporary HACK to just get it compiling as C, to match the Rich header.

In the end, we most likely want to figure out how they managed to compile C++ code into the project (either by HPP headers, or compiled objects), that did not add the C++ field (49: "Utc12_2_CPP") in the rich header.

So, yeah. Make the change and lets more forward! :)

@squidcc

This comment has been minimized.

@squidcc
Copy link
Contributor

squidcc commented Jan 7, 2019

In the end, we most likely want to figure out how they managed to compile C++ code into the project (either by HPP headers, or compiled objects), that did not add the C++ field (49: "Utc12_2_CPP") in the rich header.

Pretty sure the answer would be they were compiled using an earlier compiler build that didn't put the metadata fields in the output.
The best way to get a definitive answer would probably be to ask Patrick Wyatt (https://twitter.com/netcoyote).

@mewmew
Copy link
Contributor

mewmew commented Jan 10, 2019

To move forward, lets merge this and get the entire project compiling as C. When we figure out the deal with the C++ header, and how CRITICAL_SECTIONs were handled to get the C++ initialization while still being compiled as a C project.

@mewmew mewmew merged commit 54247a0 into diasurgical:nightly Jan 10, 2019
@AJenbo AJenbo deleted the c-init branch January 10, 2019 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants