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

rust: module: improve parsing and errors #241

Merged
merged 1 commit into from
May 1, 2021
Merged

Conversation

ojeda
Copy link
Member

@ojeda ojeda commented May 1, 2021

  • Simplified code.

  • Repeated keys are not allowed anymore, e.g.

    module! {
        type: RustMinimal,
        type: RustMinimal,
    ...
    
  • Better error messages:

    error: proc macro panicked
      --> ../samples/rust/rust_minimal.rs:10:1
       |
    10 | / module! {
    11 | |     type: RustMinimal,
    12 | |     name: b"rust_minimal",
    13 | |     description: b"Rust minimal sample",
    14 | |     author: b"Rust for Linux Contributors",
    15 | |     license: b"GPL v2",
    16 | | }
       | |_^
       |
       = help: message: Keys are not ordered as expected. Order them like: ["type", "name", "author", "description", "license"]
    
    error: proc macro panicked
      --> ../samples/rust/rust_minimal.rs:10:1
       |
    10 | / module! {
    11 | |     typo: RustMinimal,
    12 | |     name: b"rust_minimal",
    13 | |     author: b"Rust for Linux Contributors",
    14 | |     description: b"Rust minimal sample",
    15 | |     license: b"GPL v2",
    16 | | }
       | |_^
       |
       = help: message: Unknown key "typo". Valid keys are: ["type", "name", "author", "description", "license", "alias", "alias_rtnl_link", "params"].
    
    error: proc macro panicked
      --> ../samples/rust/rust_minimal.rs:10:1
       |
    10 | / module! {
    11 | |     type: RustMinimal,
    12 | |     name: b"rust_minimal",
    13 | |     author: b"Rust for Linux Contributors",
    14 | |     description: b"Rust minimal sample",
    15 | | }
       | |_^
       |
       = help: message: Missing required key "license".
    
    error: proc macro panicked
      --> ../samples/rust/rust_minimal.rs:10:1
       |
    10 | / module! {
    11 | |     type: RustMinimal,
    12 | |     name: b"rust_minimal",
    13 | |     license: b"GPL v2",
    14 | |     license: b"GPL v2",
    15 | | }
       | |_^
       |
       = help: message: Duplicated key "license". Keys can only be specified once.
    

Signed-off-by: Miguel Ojeda ojeda@kernel.org

@ojeda ojeda requested a review from kloenk May 1, 2021 16:41
@ojeda
Copy link
Member Author

ojeda commented May 1, 2021

Related: #215 and #234.

@bjorn3
Copy link
Member

bjorn3 commented May 1, 2021

error: proc macro panicked
  --> ../samples/rust/rust_minimal.rs:10:1
   |
10 | / module! {
11 | |     typo: RustMinimal,
12 | |     name: b"rust_minimal",
13 | |     author: b"Rust for Linux Contributors",
14 | |     description: b"Rust minimal sample",
15 | |     license: b"GPL v2",
16 | | }
   | |_^
   |
   = help: message: Unknown key "typo". Valid keys are: ["type", "name", "author", "description", "license", "alias", "alias_rtnl_link", "params"].

I think you can improve this error message by expanding to a compile_error!() macro invocation with the span set to that of typo.

Copy link
Member

@kloenk kloenk left a comment

Choose a reason for hiding this comment

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

LGTM.

I'm not sure if compile_error! would make it better. Probably it would make the output more readable for the specific error message, but It also would mean that the focus of the error get's shifted away from the proc_macro. So I'm fine with either ways.

@ojeda
Copy link
Member Author

ojeda commented May 1, 2021

I think you can improve this error message by expanding to a compile_error!() macro invocation with the span set to that of typo.

Tried that hack... it looks quite nice:

error: Duplicated key "type". Keys can only be specified once.
  --> ../samples/rust/rust_minimal.rs:12:5
   |
12 |     type: RustMinimal,
   |     ^^^^

Having said that, Rust should really stabilize something for proper error reporting in proc macros. Not sure if it is worth the pain at the moment...

  - Simplified code.

  - Repeated keys are not allowed anymore, e.g.

        module! {
            type: RustMinimal,
            type: RustMinimal,
        ...

  - Better error messages:

        error: proc macro panicked
          --> ../samples/rust/rust_minimal.rs:10:1
           |
        10 | / module! {
        11 | |     type: RustMinimal,
        12 | |     name: b"rust_minimal",
        13 | |     description: b"Rust minimal sample",
        14 | |     author: b"Rust for Linux Contributors",
        15 | |     license: b"GPL v2",
        16 | | }
           | |_^
           |
           = help: message: Keys are not ordered as expected. Order them like: ["type", "name", "author", "description", "license"]

        error: proc macro panicked
          --> ../samples/rust/rust_minimal.rs:10:1
           |
        10 | / module! {
        11 | |     typo: RustMinimal,
        12 | |     name: b"rust_minimal",
        13 | |     author: b"Rust for Linux Contributors",
        14 | |     description: b"Rust minimal sample",
        15 | |     license: b"GPL v2",
        16 | | }
           | |_^
           |
           = help: message: Unknown key "typo". Valid keys are: ["type", "name", "author", "description", "license", "alias", "alias_rtnl_link", "params"].

        error: proc macro panicked
          --> ../samples/rust/rust_minimal.rs:10:1
           |
        10 | / module! {
        11 | |     type: RustMinimal,
        12 | |     name: b"rust_minimal",
        13 | |     author: b"Rust for Linux Contributors",
        14 | |     description: b"Rust minimal sample",
        15 | | }
           | |_^
           |
           = help: message: Missing required key "license".

        error: proc macro panicked
          --> ../samples/rust/rust_minimal.rs:10:1
           |
        10 | / module! {
        11 | |     type: RustMinimal,
        12 | |     name: b"rust_minimal",
        13 | |     license: b"GPL v2",
        14 | |     license: b"GPL v2",
        15 | | }
           | |_^
           |
           = help: message: Duplicated key "license". Keys can only be specified once.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
@ojeda
Copy link
Member Author

ojeda commented May 1, 2021

I'm not sure if compile_error! would make it better. Probably it would make the output more readable for the specific error message, but It also would mean that the focus of the error get's shifted away from the proc_macro. So I'm fine with either ways.

Yeah, good point, I would need to mention module! in the message.

@ojeda ojeda merged commit 81fea68 into Rust-for-Linux:rust May 1, 2021
@ojeda ojeda deleted the module branch May 1, 2021 19:11
dakr pushed a commit that referenced this pull request Mar 10, 2025
With ltp test case "testcases/bin/hugefork02", there is a dmesg error
report message such as:

 kernel BUG at mm/hugetlb.c:5550!
 Oops - BUG[#1]:
 CPU: 0 UID: 0 PID: 1517 Comm: hugefork02 Not tainted 6.14.0-rc2+ #241
 Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 2/2/2022
 pc 90000000004eaf1c ra 9000000000485538 tp 900000010edbc000 sp 900000010edbf940
 a0 900000010edbfb00 a1 9000000108d20280 a2 00007fffe9474000 a3 00007ffff3474000
 a4 0000000000000000 a5 0000000000000003 a6 00000000003cadd3 a7 0000000000000000
 t0 0000000001ffffff t1 0000000001474000 t2 900000010ecd7900 t3 00007fffe9474000
 t4 00007fffe9474000 t5 0000000000000040 t6 900000010edbfb00 t7 0000000000000001
 t8 0000000000000005 u0 90000000004849d0 s9 900000010edbfa00 s0 9000000108d20280
 s1 00007fffe9474000 s2 0000000002000000 s3 9000000108d20280 s4 9000000002b38b10
 s5 900000010edbfb00 s6 00007ffff3474000 s7 0000000000000406 s8 900000010edbfa08
    ra: 9000000000485538 unmap_vmas+0x130/0x218
   ERA: 90000000004eaf1c __unmap_hugepage_range+0x6f4/0x7d0
  PRMD: 00000004 (PPLV0 +PIE -PWE)
  EUEN: 00000007 (+FPE +SXE +ASXE -BTE)
  ECFG: 00071c1d (LIE=0,2-4,10-12 VS=7)
 ESTAT: 000c0000 [BRK] (IS= ECode=12 EsubCode=0)
 PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
 Process hugefork02 (pid: 1517, threadinfo=00000000a670eaf4, task=000000007a95fc64)
 Call Trace:
 [<90000000004eaf1c>] __unmap_hugepage_range+0x6f4/0x7d0
 [<9000000000485534>] unmap_vmas+0x12c/0x218
 [<9000000000494068>] exit_mmap+0xe0/0x308
 [<900000000025fdc4>] mmput+0x74/0x180
 [<900000000026a284>] do_exit+0x294/0x898
 [<900000000026aa30>] do_group_exit+0x30/0x98
 [<900000000027bed4>] get_signal+0x83c/0x868
 [<90000000002457b4>] arch_do_signal_or_restart+0x54/0xfa0
 [<90000000015795e8>] irqentry_exit_to_user_mode+0xb8/0x138
 [<90000000002572d0>] tlb_do_page_fault_1+0x114/0x1b4

The problem is that base address allocated from hugetlbfs is not aligned
with pmd size. Here add a checking for hugetlbfs and align base address
with pmd size. After this patch the test case "testcases/bin/hugefork02"
passes to run.

This is similar to the commit 7f24cbc ("mm/mmap: teach
generic_get_unmapped_area{_topdown} to handle hugetlb mappings").

Cc: stable@vger.kernel.org  # 6.13+
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants