llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.35k stars 12.14k forks source link

LLVMDumpModule emits invalid LLVM IR, preventing diagnosis and reporting of other bugs #48702

Open andrewrk opened 3 years ago

andrewrk commented 3 years ago
Bugzilla Link 49358
Version trunk
OS All

Extended Description

Version: llvmorg-12.0.0-rc2 (4918a3d138b907a571f496661b5367e090e1e8bb)

I'm in the process of trying to report a different bug, however, LLVMDumpModule is producing invalid LLVM IR, preventing me from running other tools such as llc in order to create a reproducing test case.

For example, functions with sret are printed this way:

define internal fastcc void @​std.heap.CAllocator.alloc({ %"[]u8", i16 } nonnull sret %0, %std.builtin.StackTrace nonnull %1, %std.mem.Allocator* nonnull align 8 %2, i64 %3, i29 %4, i29 %5, i64 %6) unnamed_addr #​1 !dbg !​12261 {

Causing the following error:

[nix-shell:~/dev/zig]$ ~/local/llvm12-debug/bin/llc zig1.ll -filetype=obj /home/andy/local/llvm12-debug/bin/llc: error: /home/andy/local/llvm12-debug/bin/llc: zig1.ll:49402:87: error: expected '(' define internal fastcc void @​std.heap.CAllocator.alloc({ %"[]u8", i16 } nonnull sret %0, %std.builtin.StackTrace nonnull %1, %std.mem.Allocator* nonnull align 8 %2, i64 %3, i29 %4, i29 %5, i64 %6) unnamed_addr #​1 !dbg !​12261 {

According to the LangRef.html for LLVM 12, the syntax for sret has changed. However, LLVMDumpModule is still printing code that adheres to LLVM 11.

After this is fixed, I will have at least one, probably more, regressions to file.

andrewrk commented 3 years ago

Arguably, this should be caught by the verifier, or otherwise made easier to debug for downstream projects. However I don't see that as a reason to block the release. Closing with downstream fix: https://github.com/ziglang/zig/pull/8078

Thanks again for the help. I will run the full test suite now, will report any follow-up bugs within the next 2 days.

andrewrk commented 3 years ago

Yes it is OK. Thank you for helping me understand how to upgrade the code.

llvmbot commented 3 years ago

Oh, I noticed other uses of AttrBuilder in zig_llvm.cpp so I assumed that using it was OK. Or is that an example of what you mean by C wrapper?

andrewrk commented 3 years ago

Thanks for the clue. On the Zig side we use the C API to construct the LLVM IR, using LLVMAddAttributeAtIndex. I don't see any sret specific functions available. Does this mean we now need to create a C API wrapper for the C++ API in order to use sret?

llvmbot commented 3 years ago

The printing code does handle the type if present, so it looks like the type isn't present in the IR: https://github.com/llvm/llvm-project/blob/17517f3178b5cdda832c4e90f618437c13560013/llvm/lib/IR/AsmWriter.cpp#L4405-L4416

If sret's now require a type, I'm guessing they need to be provided from the zig side. I see some mention of sret in src/stage1/codegen.cpp, those might need to be updated to use AttrBuilder::addStructRetAttr.

andrewrk commented 3 years ago

Steps to reproduce:

git clone https://github.com/ziglang/zig git checkout llvm12

At time of reporting, the current latest commit in this branch is 70e935db5bc16e5a7144dd0de3176d53dd892301.

mkdir build cd build cmake .. -DCMAKE_PREFIX_PATH="$PATH_TO_YOUR_LLVM12_PREFIX" make zig0 ./zig0 test.zig --verbose-llvm-ir --name test --override-lib-dir ../lib -femit-bin=test.o --strip

test.zig:

const Foo = extern struct {
    a: u64,
    b: u64,
    c: u64,
    d: u64,
};

export fn entry() Foo {
    return undefined;
}

pub fn panic(msg: []const u8, error_return_trace: ?*@import("std").builtin.StackTrace) noreturn {
    unreachable;
}

This will print the following LLVM IR:

; ModuleID = 'test'
source_filename = "test"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%"[]u8" = type { i8*, i64 }
%std.builtin.StackTrace = type { i64, %"[]usize" }
%"[]usize" = type { i64*, i64 }
%Foo = type { i64, i64, i64, i64 }

@​panic.1 = internal unnamed_addr constant void (%"[]u8"*, %std.builtin.StackTrace*)* @​panic, align 8
@​output_mode = internal unnamed_addr constant i2 -2, align 1
@​0 = internal unnamed_addr constant [25 x i8] c"reached unreachable code\00", align 1
@​1 = internal unnamed_addr constant %"[]u8" { i8* getelementptr inbounds ([25 x i8], [25 x i8]* @​0, i64 0, i64 0), i64 24 }, align 8

; Function Attrs: nobuiltin noredzone noreturn nounwind
define internal fastcc void @​panic(%"[]u8"* nonnull readonly align 8 %0, %std.builtin.StackTrace* align 8 %1) unnamed_addr #​0 {
Entry:
  %error_return_trace = alloca %std.builtin.StackTrace*, align 8
  store %std.builtin.StackTrace* %1, %std.builtin.StackTrace** %error_return_trace, align 8
  call fastcc void @​panic(%"[]u8"* @​1, %std.builtin.StackTrace* null)
  unreachable
}

; Function Attrs: nobuiltin noredzone nounwind
define void @​entry(%Foo* noalias nonnull sret %0) #​1 {
Entry:
  %1 = bitcast %Foo* %0 to i8*
  call void @​llvm.memset.p0i8.i64(i8* align 8 %1, i8 -86, i64 32, i1 false)
  ret void
}

; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
declare void @​llvm.memset.p0i8.i64(i8* nocapture writeonly %0, i8 %1, i64 %2, i1 immarg %3) #​2

attributes #​0 = { nobuiltin noredzone noreturn nounwind "frame-pointer"="all" }
attributes #​1 = { nobuiltin noredzone nounwind "frame-pointer"="all" }
attributes #​2 = { argmemonly nofree nosync nounwind willreturn writeonly }

!llvm.module.flags = !{#0, !​1, !​2}
!llvm.dbg.cu = !{#3}

!​0 = !{i32 2, !"Debug Info Version", i32 3}
!​1 = !{i32 2, !"Dwarf Version", i32 4}
!​2 = !{i32 7, !"PIC Level", i32 2}
!​3 = distinct !DICompileUnit(language: DW_LANG_C99, file: !​4, producer: "zig 0.8.0", isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug, enums: !​5)
!​4 = !DIFile(filename: "test", directory: ".")
!​5 = !{#6}
!​6 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "std.builtin.OutputMode", scope: !​7, file: !​7, line: 443, baseType: !​8, size: 8, align: 8, elements: !​9)
!​7 = !DIFile(filename: "builtin.zig", directory: "/home/andy/dev/zig/lib/std")
!​8 = !DIBasicType(name: "u2", size: 8, encoding: DW_ATE_unsigned)
!​9 = !{#10, !​11, !​12}
!​10 = !DIEnumerator(name: "Exe", value: 0)
!​11 = !DIEnumerator(name: "Lib", value: 1)
!​12 = !DIEnumerator(name: "Obj", value: 2)

The problem is observed on this line:

define void @​entry(%Foo* noalias nonnull sret %0) #​1 {

As demonstrated by llc:

[nix-shell:~/dev/zig/build-llvm12]$ ~/local/llvm12-debug/bin/llc test.ll -filetype=obj /home/andy/local/llvm12-debug/bin/llc: error: /home/andy/local/llvm12-debug/bin/llc: test.ll:26:47: error: expected '(' define void @​entry(%Foo* noalias nonnull sret %0) #​1 { ^