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

Update s2e-core v8 #102

Merged
merged 26 commits into from
Dec 12, 2024
Merged

Update s2e-core v8 #102

merged 26 commits into from
Dec 12, 2024

Conversation

200km
Copy link
Member

@200km 200km commented Dec 11, 2024

Issue

NA

詳細

s2e-core v8に対応したアップデート
基本的には、v8で修正されたnamespaceの追加、ディレクトリの変更に対応しただけでアルゴリズムの修正は行なっていない。

検証結果

CI結果参照

補足

NA

@200km 200km added 🚀 priority::high priority high 🐳 major update Major update labels Dec 11, 2024
@200km 200km added this to the Major update v6.0.0 milestone Dec 11, 2024
@200km 200km self-assigned this Dec 11, 2024
@200km 200km requested review from sksat and a team as code owners December 11, 2024 15:33
@200km 200km requested review from ogoogo, Hiro-0110, seki-hiro, suzuki-toshihir0, t-hosonuma and conjikidow and removed request for a team December 11, 2024 15:33
@200km
Copy link
Member Author

200km commented Dec 11, 2024

s2e-core側で下記の場所でstd::stringからfs::pathにキャストされているところでエラーが出ているっぽい?

https://github.com/ut-issl/s2e-core/blob/8361399b9ba53b84215f7a5ac1d09aa6f6ef28a6/src/simulation/case/simulation_case.cpp#L35

@200km
Copy link
Member Author

200km commented Dec 11, 2024

windowsだけの問題なのも下記の記述と一致する?

https://stackoverflow.com/questions/43114174/convert-a-string-to-std-filesystem-path

@200km
Copy link
Member Author

200km commented Dec 11, 2024

c2a ONでエラーできるのは下記のc2a-aobc側で対処
ut-issl/c2a-aobc#332

@200km 200km mentioned this pull request Dec 11, 2024
@200km
Copy link
Member Author

200km commented Dec 11, 2024

もしくは、下記のnamespaceを削除した方が良いのか?

https://github.com/ut-issl/s2e-core/blob/develop/src/components/real/cdh/c2a_communication.hpp

@conjikidow
Copy link
Member

対応ありがとうございます。

もしくは、下記のnamespaceを削除した方が良いのか?

https://github.com/ut-issl/s2e-core/blob/develop/src/components/real/cdh/c2a_communication.hpp

悩ましいですが,このnamespaceがあると,c2aを読んでいるときにこれらの関数がs2e側で実装されているということが一目でわかるため,個人的にはこのままで良いかなと思います。

@200km
Copy link
Member Author

200km commented Dec 12, 2024

c2aを読んでいるときにこれらの関数がs2e側で実装されているということが一目でわかるため

これを今の時点で目指すか、そもそもc2aないのIfWrapper/SilsはS2Eにあった方がよいのでは?とかの議論も含めてより良い形に将来するために今は、namespaceを削除して簡単に済ませるのかは考えたいですね。

@conjikidow
Copy link
Member

そもそもc2aないのIfWrapper/SilsはS2Eにあった方がよいのでは?とかの議論も含めてより良い形に将来する

こういう可能性もあるのであれば,とりあえず今はnamespaceはつけなくてもいいかもしれないですね。

Copy link
Member

@conjikidow conjikidow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。
とりあえずC2Aなしについては動作確認も取れました。
C2Aありは上の方針が決まってから改めて確認します。

@200km
Copy link
Member Author

200km commented Dec 12, 2024

C2A ONでビルドが通らない件に関して、s2e-core側の修正しました。

ut-issl/s2e-core#724

@200km
Copy link
Member Author

200km commented Dec 12, 2024

s2e-coreを修正することで、gccについてはC2A ONでCI通ることが確認できた。

@200km
Copy link
Member Author

200km commented Dec 12, 2024

windowsの方も治った!

@200km 200km requested a review from conjikidow December 12, 2024 14:38
Copy link
Member

@conjikidow conjikidow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます!

@200km 200km merged commit 3fbed0b into develop Dec 12, 2024
6 checks passed
@200km 200km deleted the feature/update-s2e-core-v8 branch December 12, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants