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 breaks with gcc 11.3.1 #6702

Merged
merged 2 commits into from
Sep 13, 2022
Merged

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Sep 12, 2022

  • Fix documentation to ensure OMR_GC_MODRON_SCAVENGER is enabled during the build
  • Add -O3 to omrtrace component to prevent warning about not compiling with -O when _FORTIFY_SOURCE is specified.

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 12, 2022

@keithc-ca @babsingh could you please review?

@babsingh
Copy link
Contributor

We should be able to build OMR without OMR_GC_MODRON_SCAVENGER. It should not be a mandatory requirement. So, the below error should be fixed by either consistently using the same ifdef macros or enabling dependent ifdef macros correctly. MarkingScheme.hpp:292 uses OMR_GC_CONCURRENT_SCAVENGER whereas GCExtensionsBase.hpp uses OMR_GC_MODRON_SCAVENGER for isScavengerBackOutFlagRaised. fyi @dmitripivkine @amicic @LinHu2016

/home/harpreet/omr/gc/base/MarkingScheme.hpp:292:67: error: 'class MM_GCExtensionsBase' has no member named 'isScavengerBackOutFlagRaised'; did you mean 'isScavengerEnabled'?
   if (_extensions->isConcurrentScavengerEnabled() && _extensions->isScavengerBackOutFlagRaised()) {
                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                                   isScavengerEnabled

@dmitripivkine
Copy link
Contributor

We should be able to build OMR without OMR_GC_MODRON_SCAVENGER. It should not be a mandatory requirement. So, the below error should be fixed by either consistently using the same ifdef macros or enabling dependent ifdef macros correctly. MarkingScheme.hpp:292 uses OMR_GC_CONCURRENT_SCAVENGER whereas GCExtensionsBase.hpp uses OMR_GC_MODRON_SCAVENGER for isScavengerBackOutFlagRaised. fyi @dmitripivkine @amicic @LinHu2016

/home/harpreet/omr/gc/base/MarkingScheme.hpp:292:67: error: 'class MM_GCExtensionsBase' has no member named 'isScavengerBackOutFlagRaised'; did you mean 'isScavengerEnabled'?
   if (_extensions->isConcurrentScavengerEnabled() && _extensions->isScavengerBackOutFlagRaised()) {
                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                                   isScavengerEnabled

Yes, if should be fixed eventually. However I have suspicion the condition check was missed at time of split code for OMR many years ago. I think there are more missed checks like this in the code (not only Scavenger but another GC features related). So OMR code requires inspection/cleaning project. Also when it completes it required testing support - compiling code with disabled features on regular basis. Otherwise new errors are going to be introduced unavoidably. Currently we have no resources to address this, so, this is another issue for backlog

@babsingh
Copy link
Contributor

babsingh commented Sep 13, 2022

LGTM. GC fix is a good temporary workaround. -O3 is only set when _FORTIFY_SOURCE is enabled on Linux. @keithc-ca Do these change look good to you?

README.md Outdated Show resolved Hide resolved
The current build instructions results in a build break in the gc
component. This PR updates the build instructions to ensure that
OMR_GC_MODRON_SCAVENGER is enabled at build time.

Signed-off-by: Irwin D'Souza <[email protected]>
This commit adds -O3 to the CMakeLists.txt of the omrtrace component to
prevent the following warning treated as an error:

warning _FORTIFY_SOURCE requires compiling with optimization (-O)

Signed-off-by: Irwin D'Souza <[email protected]>
@babsingh
Copy link
Contributor

jenkins build all

To make sure, the -O3 change works fine on all Linux platforms.

@babsingh
Copy link
Contributor

https://ci.eclipse.org/omr/job/PullRequest-win_x86-64/3469/console

@AdamBrousseau Noticed a new infra issue on Windows, which is unrelated to this PR. Is this a known infra issue?

@AdamBrousseau
Copy link
Contributor

jenkins build win

@AdamBrousseau
Copy link
Contributor

We were testing a new machine. Back to normal now.

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 13, 2022

riscv build failed because of:

11:23:11  20: [----------] 18 tests from RWMutex
13:05:04  Cancelling nested steps due to timeout
13:05:04  Sending interrupt signal to process
13:05:24  After 20s process did not stop

It's not the first time there's been a hang in RWMutex (see #277 (comment)) though that was all the way in 2016 on OSX, so kind of a stretch to link these two.

Will run the test again to see if it's reproducable.

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 13, 2022

jenkins build riscv64

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 13, 2022

Don't see the RWMutex hang this time around; riscv64 failure now because of #6704

@babsingh
Copy link
Contributor

The failures seen in the PR builds are unrelated to the changes in this PR: #6704 and #6706. Thus, merging.

@babsingh babsingh merged commit 7b747ec into eclipse-omr:master Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants