Project

General

Profile

Actions

Bug #504

closed

MingW meson build is broken with pre-2019 libraries

Added by Anastasia Klimchuk 11 months ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Category:
-
Target version:
Start date:
08/10/2023
Due date:
% Done:

0%

Estimated time:
Affected versions:
Needs backport to:
Affected hardware:
Affected OS:

Description

At head of master branch:

Logs building with meson: https://pastebin.com/LgWPUkBC

Makefile builds and works fine


Files

rebuild.txt (15.4 KB) rebuild.txt Miklos Márton, 08/19/2023 09:33 AM
Actions #1

Updated by Anastasia Klimchuk 11 months ago

One more question, is how to test WP functionality on MX25L1605E

Actions #2

Updated by Miklos Márton 11 months ago

  • Subject changed from NI-845x programmer meson build is broken to MingW meson build is broken
Actions #3

Updated by Peter Marheine 11 months ago

Copying output from the link:

mm@DESKTOP-4AITTRD /z/Projektek/flashrom_git
$ meson compile -C builddir
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: "c:\Program Files\Meson\ninja.EXE" -C //VBoxSvr/mm/Projektek/flashrom_git/builddir
ninja: Entering directory `//VBoxSvr/mm/Projektek/flashrom_git/builddir'
[7/116] Compiling C object libflashrom-1.dll.p/helpers_fileio.c.obj
FAILED: libflashrom-1.dll.p/helpers_fileio.c.obj
"gcc" "-Ilibflashrom-1.dll.p" "-I." "-I.." "-I..\include" "-IC:\Program Files (x86)\National Instruments\Ni-845x\MS Visual C" "-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-Wextra" "-Werror" "-std=c99" "-Os" "-Wshadow" "-Wmissing-prototypes" "-Wwrite-strings" "-Wno-unused-parameter" "-Wno-missing-braces" "-D_DEFAULT_SOURCE" "-D_POSIX_C_SOURCE=200809L" "-D_BSD_SOURCE" "-D__BSD_VISIBLE" "-D__XSI_VISIBLE" "-D_NETBSD_SOURCE" "-D_DARWIN_C_SOURCE" "-DFLASHROM_VERSION=\"1.4.0-devel (git:v1.2-1336-gd666a818)\"" "-DHAVE_STRNLEN=1" "-DIS_WINDOWS=1" "-D__FLASHROM_LITTLE_ENDIAN__=1" "-DCONFIG_NI845X_SPI=1" "-DCONFIG_DEFAULT_PROGRAMMER_NAME=NULL" "-DCONFIG_DEFAULT_PROGRAMMER_ARGS=\"\"" -MD -MQ libflashrom-1.dll.p/helpers_fileio.c.obj -MF "libflashrom-1.dll.p\helpers_fileio.c.obj.d" -o libflashrom-1.dll.p/helpers_fileio.c.obj "-c" ../helpers_fileio.c
../helpers_fileio.c: In function 'read_buf_from_file':
../helpers_fileio.c:41:11: error: implicit declaration of function 'fdopen' [-Werror=implicit-function-declaration]
   image = fdopen(fileno(stdin), "rb");
           ^~~~~~
../helpers_fileio.c:41:18: error: implicit declaration of function 'fileno' [-Werror=implicit-function-declaration]
   image = fdopen(fileno(stdin), "rb");
                  ^~~~~~
../helpers_fileio.c:41:9: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   image = fdopen(fileno(stdin), "rb");
         ^
cc1.exe: all warnings being treated as errors
[12/116] Compiling C object libflashrom-1.dll.p/flashrom.c.obj
FAILED: libflashrom-1.dll.p/flashrom.c.obj
"gcc" "-Ilibflashrom-1.dll.p" "-I." "-I.." "-I..\include" "-IC:\Program Files (x86)\National Instruments\Ni-845x\MS Visual C" "-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-Wextra" "-Werror" "-std=c99" "-Os" "-Wshadow" "-Wmissing-prototypes" "-Wwrite-strings" "-Wno-unused-parameter" "-Wno-missing-braces" "-D_DEFAULT_SOURCE" "-D_POSIX_C_SOURCE=200809L" "-D_BSD_SOURCE" "-D__BSD_VISIBLE" "-D__XSI_VISIBLE" "-D_NETBSD_SOURCE" "-D_DARWIN_C_SOURCE" "-DFLASHROM_VERSION=\"1.4.0-devel (git:v1.2-1336-gd666a818)\"" "-DHAVE_STRNLEN=1" "-DIS_WINDOWS=1" "-D__FLASHROM_LITTLE_ENDIAN__=1" "-DCONFIG_NI845X_SPI=1" "-DCONFIG_DEFAULT_PROGRAMMER_NAME=NULL" "-DCONFIG_DEFAULT_PROGRAMMER_ARGS=\"\"" -MD -MQ libflashrom-1.dll.p/flashrom.c.obj -MF "libflashrom-1.dll.p\flashrom.c.obj.d" -o libflashrom-1.dll.p/flashrom.c.obj "-c" ../flashrom.c
../flashrom.c: In function 'programmer_init':
../flashrom.c:162:16: error: implicit declaration of function 'strdup' [-Werror=implicit-function-declaration]
   cfg.params = strdup(param);
                ^~~~~~
../flashrom.c:162:14: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   cfg.params = strdup(param);
              ^
../flashrom.c: In function 'get_flash_region':
../flashrom.c:380:16: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   region->name = strdup("");
                ^
../flashrom.c: In function 'flashbuses_to_text':
../flashrom.c:939:10: error: return makes pointer from integer without a cast [-Werror=int-conversion]
   return strdup("Non-SPI");
          ^~~~~~~~~~~~~~~~~
../flashrom.c:941:10: error: return makes pointer from integer without a cast [-Werror=int-conversion]
   return strdup("None");
          ^~~~~~~~~~~~~~
cc1.exe: all warnings being treated as errors
ninja: build stopped: subcommand failed.

I'll see if I can reproduce, but probably need more information on the environment being used here; it might be a very old compiler or something.

Actions #4

Updated by Anastasia Klimchuk 11 months ago

Don't know whether it's useful or not, but I recalled a patch which was adding missing includes: https://review.coreboot.org/c/flashrom/+/75235

could it be something similar in this case (missing includes)?

Actions #5

Updated by Peter Marheine 11 months ago

Yeah, that's all it looks like to me.

Actions #6

Updated by Anastasia Klimchuk 11 months ago

Miklos, could you please give us more info about your environment?

I know meson builds lots of info when it does a full re-build, but I don't see it in your output that you pasted. Maybe do a small change in the code and re-build? It will print:

[0/1] Regenerating build files.
The Meson build system
Version: 0.61.2
and then ~40 more lines about the environment on the machine.

That would be very useful.

Actions #7

Updated by Peter Marheine 11 months ago

Here's what my machine says when I build ni845x_spi only:

$ git checkout master
HEAD is now at d666a8189 doc: Fix broken link to old mailing list archives on pipermail
$ meson setup build -Dprogrammer=ni845x_spi
The Meson build system
Version: 1.1.1
Source dir: C:/Users/pmarheine/Desktop/flashrom
Build dir: C:/Users/pmarheine/Desktop/flashrom/build
Build type: native build
Project name: flashromutils
Project version: 1.4.0-devel
C compiler for the host machine: cc (gcc 13.1.0 "cc (Rev6, Built by MSYS2 project) 13.1.0")
C linker for the host machine: cc ld.bfd 2.40
Host machine cpu family: x86
Host machine cpu: x86
Program git found: YES (C:\msys64\usr\bin/git.EXE)
Program sphinx-build found: YES (C:\msys64\mingw32\bin/sphinx-build.EXE)
Compiler for C supports arguments -Wshadow: YES
Compiler for C supports arguments -Wmissing-prototypes: YES
Compiler for C supports arguments -Wwrite-strings: YES
Compiler for C supports arguments -Wno-unused-parameter: YES
Compiler for C supports arguments -Wno-address-of-packed-member: YES
Compiler for C supports arguments -Wno-enum-conversion: YES
Compiler for C supports arguments -Wno-missing-braces: YES
Checking for function "clock_gettime" : YES
Checking for function "strnlen" : YES
Check usable header "sys/utsname.h" : NO
Found pkg-config: C:\msys64\mingw32\bin/pkg-config.EXE (0.29.2)
Did not find CMake 'cmake'
Found CMake: NO
Run-time dependency libpci found: NO (tried pkgconfig and cmake)
Run-time dependency libusb-1.0 found: YES 1.0.26
Run-time dependency libftdi1 found: YES 1.5
Dependency libjaylink found: NO found 0.2.0 but need: '>=0.3.0'
Run-time dependency libjaylink found: NO (tried pkgconfig and cmake)
Library ni845x found: YES
Has header "linux/i2c.h" : NO
Checking for function "getopt_long" : YES
Run-time dependency bash-completion found: NO (tried pkgconfig and cmake)
Configuring flashrom.bash using configuration
Run-time dependency cmocka found: YES 1.1.5
Run-time dependency threads found: YES
Build targets in project: 7

flashromutils 1.4.0-devel

  Programmer
    active    : ni845x_spi
    non active: asm106x (not selected)
                atahpt (not selected)
                atapromise (not selected)
                atavia (not selected)
                buspirate_spi (not selected)
                ch341a_spi (not selected)
                ch347_spi (not selected)
                dediprog (not selected)
                developerbox_spi (not selected)
                digilent_spi (not selected)
                dirtyjtag_spi (not selected)
                drkaiser (not selected)
                dummy (not selected)
                ft2232_spi (not selected)
                gfxnvidia (not selected)
                internal (not selected)
                it8212 (not selected)
                jlink_spi (not selected)
                linux_mtd (not selected)
                linux_spi (not selected)
                parade_lspcon (not selected)
                mediatek_i2c_spi (not selected)
                mstarddc_spi (not selected)
                nic3com (not selected)
                nicintel (not selected)
                nicintel_eeprom (not selected)
                nicintel_spi (not selected)
                nicnatsemi (not selected)
                nicrealtek (not selected)
                ogp_spi (not selected)
                pickit2_spi (not selected)
                pony_spi (not selected)
                raiden_debug_spi (not selected)
                rayer_spi (not selected)
                realtek_mst_i2c_spi (not selected)
                satamv (not selected)
                satasii (not selected)
                serprog (not selected)
                stlinkv3_spi (not selected)
                usbblaster_spi (not selected)

  User defined options
    programmer: ni845x_spi

Found ninja-1.11.1 at C:\msys64\mingw32\bin/ninja.EXE
$ meson compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: C:\msys64\mingw32\bin/ninja.EXE -C C:/Users/pmarheine/Desktop/flashrom/build
ninja: Entering directory `C:/Users/pmarheine/Desktop/flashrom/build'
[115/115] Linking target tests/flashrom_unit_tests.exe

Using the default programmers (same procedure, but omit -Dprogrammer=...) also builds fine.

Actions #8

Updated by Miklos Márton 11 months ago

Anastasia Klimchuk wrote in #note-6:

Miklos, could you please give us more info about your environment?

I know meson builds lots of info when it does a full re-build, but I don't see it in your output that you pasted. Maybe do a small change in the code and re-build? It will print:

[0/1] Regenerating build files.
The Meson build system
Version: 0.61.2
and then ~40 more lines about the environment on the machine.

That would be very useful.

I have removed the builddir and ran a meson setup builddir meson compile -C builddir

My Mingw installation version is 6.3.0

Actions #9

Updated by Peter Marheine 11 months ago

gcc (MinGW.org GCC-6.3.0-1) 6.3.0 is a very old compiler, and the "MinGW.org" line further supports that theory (that site no longer exists). The current documentation says to use msys2 which ships a much newer compiler (my environment is representative of that), so either you need to update your tools; or (if we decide that these old tools should still be supported) there need to be some code changes to work around the differences with an old standard library.

Actions #10

Updated by Anastasia Klimchuk 11 months ago

I also noticed the difference in two environments: in one of the env libusb and libftdi are not present.
Peter, is it possible that you try to uninstall these two libraries? maybe that would repro even with new compiler?

Actions #11

Updated by Peter Marheine 11 months ago

Build still succeeds for me after removing libusb, libftdi and libjaylink; confirmed that meson output says they were not found and many of the programmers were disabled since they need those libraries.

Actions #12

Updated by Anastasia Klimchuk 11 months ago

I am very confused, because all the includes seem to be present already.

fdopen and fileno are included with stdio.h
strdup is included in string.h

helpers_fileio.c includes "flash.h" -> which includes stdio.h
flashrom.c includes string.h (and also includes stdio.h)

Also in Miklos's logs, cmocka logs that both headers are present (check usable headers YES).

I am very curious how make builds it?

Actions #13

Updated by Anastasia Klimchuk 11 months ago

I think I found something.

There is an extra bit in Makefile which meson.build does not have, conditional for target OS MinGW:

# Some functions provided by Microsoft do not work as described in C99 specifications. This macro fixes that
# for MinGW. See http://sourceforge.net/p/mingw-w64/wiki2/printf%20and%20scanf%20family/ */
FLASHROM_CFLAGS += -D__USE_MINGW_ANSI_STDIO=1

I searched about __USE_MINGW_ANSI_STDIO and it seems to be deprecated? (thread from Jan 2019) https://osdn.net/projects/mingw/lists/archive/users/2019-January/000199.html

This would explain why the newer compiler works (because it knows about deprecation) and older one does not.

Miklos, as an experiment, you can try running meson with __USE_MINGW_ANSI_STDIO force enabled?
Hopefully this will do it:

meson configure builddir -D__USE_MINGW_ANSI_STDIO=1
or
meson setup builddir -D__USE_MINGW_ANSI_STDIO=1

In general, if the option is actually deprecated, I don't think we should be adding it to meson build?

Is it possible you would try to build with a newer compiler, as Peter suggested?

Thank you!

Actions #14

Updated by Peter Marheine 11 months ago

Nice sleuthing! I'd prefer to simply say that we now only support reasonably new compilers, but it seems potentially more correct to define the appropriate feature_test_macros(7). https://mingw-users.narkive.com/tAeczSOC/snprintf-and-use-mingw-ansi-stdio#post2 says you should never manually define __USE_MINGW_ANSI_STDIO and should use the appropriate feature test macros instead.

This turns out to be something I habitually skim right over in manpages, but it looks like always defining _XOPEN_SOURCE=500 is appropriate: the problematic functions identified earlier are fdopen, fileno and strdup. fdopen and fileno require _POSIX_C_SOURCE, and strdup requires (among other options) _XOPEN_SOURCE>=500. The glibc documentation says setting _XOPEN_SOURCE to a value 500 or greater implies _POSIX_C_SOURCE, so just setting _XOPEN_SOURCE is sufficient.

Setting this value expresses an expectation that the environment provides compliance with Single UNIX Specification v2 (1997), which seems like a reasonable assumption at this time: we already depend on these specifications by virtue of using those functions, and each of them is more than 20 years old at this time so I don't think it's worth bothering to support systems that fail to provide these features.

Actions #15

Updated by Peter Marheine 11 months ago

  • Subject changed from MingW meson build is broken to MingW meson build is broken with pre-2019 libraries
Actions #16

Updated by Peter Marheine 11 months ago

On further investigation, we already set _POSIX_C_SOURCE=200809 which should provide fdopen and fileno, so we can put the blame for those on MinGW (which seems like it has been fixed at some point). Pre-2.12 versions of glibc require _XOPEN_SOURCE >= 500 which we don't currently do, but that version was released in 2010 so I don't think it's worth supporting.

Actions #17

Updated by Peter Marheine 11 months ago

Missed a word: pre-2.12 versions of glibc require _XOPEN_SOURCE for strdup.

Actions #18

Updated by Anastasia Klimchuk 11 months ago

Peter Marheine wrote in #note-14:

it looks like always defining _XOPEN_SOURCE=500 is appropriate

Would that be a patch to meson.build (or tell me, maybe I am misunderstanding you)? There are few places which detect host_machine.system() , does this need to be added into one of those?
Is it useful to add this info into docs as well?

Actions #19

Updated by Peter Marheine 11 months ago

I meant in meson.build, but we already define a bunch of feature selection macros that should make the required features available. The problem is only that old versions of MinGW tools don't correctly handle those feature test macros to enable the requisite items, so we'd have to use __USE_MINGW_ANSI_STDIO if we want to support these old tools (and I don't think it's worth doing that).

Actions #20

Updated by Peter Marheine 9 months ago

  • Status changed from New to Closed

As there have been no other opinions on whether we should try to support sufficiently old MinGW, I'm closing this since I don't think it's worth supporting.

Actions

Also available in: Atom PDF