* [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072]
@ 2022-12-14 9:14 Jakub Jelinek
2022-12-14 10:04 ` Arthur Cohen
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2022-12-14 9:14 UTC (permalink / raw)
To: gcc-rust, gcc-patches
Hi!
Bootstrap fails on aarch64-linux and some other arches with:
.../gcc/rust/parse/rust-parse-impl.h: In member function ‘Rust::AST::ClosureParam Rust::Parser<ManagedTokenSource>::parse_closure_param() [with ManagedTokenSource = Rust::Lexer]’:
.../gcc/rust/parse/rust-parse-impl.h:8916:49: error: ‘this’ pointer is null [-Werror=nonnull]
The problem is that while say on x86_64-linux the side-effects in the
arguments are evaluated from last argument to first, on aarch64-linux
it is the other way around, from first to last. The C++ I believe even
in C++17 makes the evaluation of those side-effects unordered
(indeterminately sequenced with no interleaving), so that is fine.
But, when the call in return statement is evaluated from first to
last, std::move (pattern) happens before pattern->get_locus () and
the former will make pattern (std::unique_ptr) a wrapper object around
nullptr, so dereferencing it later to call get_locus () on it is invalid.
The following patch fixes that, ok for trunk?
2022-12-14 Jakub Jelinek <jakub@redhat.com>
PR rust/106072
* parse/rust-parse-impl.h (parse_closure_param): Store
pattern->get_locus () in a temporary before std::move (pattern) is
invoked.
--- gcc/rust/parse/rust-parse-impl.h.jj 2022-12-13 16:50:12.708093521 +0100
+++ gcc/rust/parse/rust-parse-impl.h 2022-12-14 09:50:31.731111932 +0100
@@ -8912,8 +8912,9 @@ Parser<ManagedTokenSource>::parse_closur
}
}
- return AST::ClosureParam (std::move (pattern), pattern->get_locus (),
- std::move (type), std::move (outer_attrs));
+ Location loc = pattern->get_locus ();
+ return AST::ClosureParam (std::move (pattern), loc, std::move (type),
+ std::move (outer_attrs));
}
// Parses a grouped or tuple expression (disambiguates).
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072]
2022-12-14 9:14 [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072] Jakub Jelinek
@ 2022-12-14 10:04 ` Arthur Cohen
2022-12-14 10:38 ` Kyrylo Tkachov
0 siblings, 1 reply; 3+ messages in thread
From: Arthur Cohen @ 2022-12-14 10:04 UTC (permalink / raw)
To: Jakub Jelinek, gcc-rust, gcc-patches
[-- Attachment #1.1.1: Type: text/plain, Size: 2788 bytes --]
Hi Jakub,
On 12/14/22 10:14, Jakub Jelinek via Gcc-rust wrote:
> Hi!
>
> Bootstrap fails on aarch64-linux and some other arches with:
> .../gcc/rust/parse/rust-parse-impl.h: In member function ‘Rust::AST::ClosureParam Rust::Parser<ManagedTokenSource>::parse_closure_param() [with ManagedTokenSource = Rust::Lexer]’:
> .../gcc/rust/parse/rust-parse-impl.h:8916:49: error: ‘this’ pointer is null [-Werror=nonnull]
> The problem is that while say on x86_64-linux the side-effects in the
> arguments are evaluated from last argument to first, on aarch64-linux
> it is the other way around, from first to last. The C++ I believe even
> in C++17 makes the evaluation of those side-effects unordered
> (indeterminately sequenced with no interleaving), so that is fine.
> But, when the call in return statement is evaluated from first to
> last, std::move (pattern) happens before pattern->get_locus () and
> the former will make pattern (std::unique_ptr) a wrapper object around
> nullptr, so dereferencing it later to call get_locus () on it is invalid.
>
> The following patch fixes that, ok for trunk?
>
> 2022-12-14 Jakub Jelinek <jakub@redhat.com>
>
> PR rust/106072
> * parse/rust-parse-impl.h (parse_closure_param): Store
> pattern->get_locus () in a temporary before std::move (pattern) is
> invoked.
>
> --- gcc/rust/parse/rust-parse-impl.h.jj 2022-12-13 16:50:12.708093521 +0100
> +++ gcc/rust/parse/rust-parse-impl.h 2022-12-14 09:50:31.731111932 +0100
> @@ -8912,8 +8912,9 @@ Parser<ManagedTokenSource>::parse_closur
> }
> }
>
> - return AST::ClosureParam (std::move (pattern), pattern->get_locus (),
> - std::move (type), std::move (outer_attrs));
> + Location loc = pattern->get_locus ();
> + return AST::ClosureParam (std::move (pattern), loc, std::move (type),
> + std::move (outer_attrs));
> }
>
> // Parses a grouped or tuple expression (disambiguates).
>
> Jakub
>
Thanks :) this looks good to me. We already have that issue fixed in our
upstream dev branch, by this PR:
https://github.com/Rust-GCC/gccrs/pull/1619
but we have yet to update GCC's master with our upstream dev branch, so
in the meantime feel free to apply your patch. When I'll get to updating
master, I'm expecting these kinds of tiny conflicts and we'll deal with
them.
Thanks a lot for working on this and sorry that my tardiness in updating
has caused a duplication of efforts.
All the best,
--
Arthur Cohen <arthur.cohen@embecosm.com>
Toolchain Engineer
Embecosm GmbH
Geschäftsführer: Jeremy Bennett
Niederlassung: Nürnberg
Handelsregister: HR-B 36368
www.embecosm.de
Fürther Str. 27
90429 Nürnberg
Tel.: 091 - 128 707 040
Fax: 091 - 128 707 077
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3195 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072]
2022-12-14 10:04 ` Arthur Cohen
@ 2022-12-14 10:38 ` Kyrylo Tkachov
0 siblings, 0 replies; 3+ messages in thread
From: Kyrylo Tkachov @ 2022-12-14 10:38 UTC (permalink / raw)
To: Arthur Cohen, Jakub Jelinek, gcc-rust, gcc-patches
> -----Original Message-----
> From: Gcc-patches <gcc-patches-
> bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Arthur Cohen
> Sent: Wednesday, December 14, 2022 10:05 AM
> To: Jakub Jelinek <jakub@redhat.com>; gcc-rust@gcc.gnu.org; gcc-
> patches@gcc.gnu.org
> Subject: Re: [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072]
>
> Hi Jakub,
>
> On 12/14/22 10:14, Jakub Jelinek via Gcc-rust wrote:
> > Hi!
> >
> > Bootstrap fails on aarch64-linux and some other arches with:
> > .../gcc/rust/parse/rust-parse-impl.h: In member function
> ‘Rust::AST::ClosureParam
> Rust::Parser<ManagedTokenSource>::parse_closure_param() [with
> ManagedTokenSource = Rust::Lexer]’:
> > .../gcc/rust/parse/rust-parse-impl.h:8916:49: error: ‘this’ pointer is null [-
> Werror=nonnull]
> > The problem is that while say on x86_64-linux the side-effects in the
> > arguments are evaluated from last argument to first, on aarch64-linux
> > it is the other way around, from first to last. The C++ I believe even
> > in C++17 makes the evaluation of those side-effects unordered
> > (indeterminately sequenced with no interleaving), so that is fine.
> > But, when the call in return statement is evaluated from first to
> > last, std::move (pattern) happens before pattern->get_locus () and
> > the former will make pattern (std::unique_ptr) a wrapper object around
> > nullptr, so dereferencing it later to call get_locus () on it is invalid.
> >
> > The following patch fixes that, ok for trunk?
FWIW, with this patch my aarch64 bootstrap progressed past the previous point of failure (it's currently in stage 3).
Thanks,
Kyrill
> >
> > 2022-12-14 Jakub Jelinek <jakub@redhat.com>
> >
> > PR rust/106072
> > * parse/rust-parse-impl.h (parse_closure_param): Store
> > pattern->get_locus () in a temporary before std::move (pattern) is
> > invoked.
> >
> > --- gcc/rust/parse/rust-parse-impl.h.jj 2022-12-13
> 16:50:12.708093521 +0100
> > +++ gcc/rust/parse/rust-parse-impl.h 2022-12-14 09:50:31.731111932
> +0100
> > @@ -8912,8 +8912,9 @@ Parser<ManagedTokenSource>::parse_closur
> > }
> > }
> >
> > - return AST::ClosureParam (std::move (pattern), pattern->get_locus (),
> > - std::move (type), std::move (outer_attrs));
> > + Location loc = pattern->get_locus ();
> > + return AST::ClosureParam (std::move (pattern), loc, std::move (type),
> > + std::move (outer_attrs));
> > }
> >
> > // Parses a grouped or tuple expression (disambiguates).
> >
> > Jakub
> >
>
> Thanks :) this looks good to me. We already have that issue fixed in our
> upstream dev branch, by this PR:
>
> https://github.com/Rust-GCC/gccrs/pull/1619
>
> but we have yet to update GCC's master with our upstream dev branch, so
> in the meantime feel free to apply your patch. When I'll get to updating
> master, I'm expecting these kinds of tiny conflicts and we'll deal with
> them.
>
> Thanks a lot for working on this and sorry that my tardiness in updating
> has caused a duplication of efforts.
>
> All the best,
>
> --
> Arthur Cohen <arthur.cohen@embecosm.com>
>
> Toolchain Engineer
>
> Embecosm GmbH
>
> Geschäftsführer: Jeremy Bennett
> Niederlassung: Nürnberg
> Handelsregister: HR-B 36368
> www.embecosm.de
>
> Fürther Str. 27
> 90429 Nürnberg
>
>
> Tel.: 091 - 128 707 040
> Fax: 091 - 128 707 077
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-14 10:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-14 9:14 [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072] Jakub Jelinek
2022-12-14 10:04 ` Arthur Cohen
2022-12-14 10:38 ` Kyrylo Tkachov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).