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

build fails on AIX #374

Closed
arozehnal opened this issue Mar 18, 2024 · 12 comments
Closed

build fails on AIX #374

arozehnal opened this issue Mar 18, 2024 · 12 comments
Labels
patch A patch to fix an issue problem Something isn't working due to a (minor) problem

Comments

@arozehnal
Copy link

mmap.hpp: In member function 'bool MMap::file(reflex::Input&, const char*&, size_t&)':
mmap.hpp:113:17: error: invalid conversion from 'void*' to 'caddr_t' {aka 'char*'} [-fpermissive]
113 | madvise(mmap_base, mmap_size, MADV_SEQUENTIAL);
| ^~~~~~~~~
| |
| void*
In file included from mmap.hpp:51,
from ugrep.cpp:79:
/usr/include/sys/mman.h:189:22: note: initializing argument 1 of 'int madvise(caddr_t, size_t, int)'
189 | extern int madvise(caddr_t, size_t, int);
| ^~~~~~~
ugrep.cpp: In function 'void init(int, const char**)':
ugrep.cpp:6851:33: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
6851 | include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ugrep.cpp:6858:35: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
6858 | include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ugrep.cpp:6875:35: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
6875 | exclude_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ugrep.cpp:6897:31: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
6897 | include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ugrep.cpp:6913:35: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
6913 | include_fs_ids.insert(static_cast<uint64_t>(buf.f_fsid));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mv -f .deps/ugrep-stats.Tpo .deps/ugrep-stats.Po
ugrep.cpp: In member function 'virtual void Grep::recurse(size_t, const char*)':
ugrep.cpp:9252:21: error: invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}
9252 | uint64_t id = static_cast<uint64_t>(buf.f_fsid);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mv -f .deps/ugrep-vkey.Tpo .deps/ugrep-vkey.Po
mv -f .deps/ugrep-screen.Tpo .deps/ugrep-screen.Po
mv -f .deps/ugrep-cnf.Tpo .deps/ugrep-cnf.Po
make[2]: *** [Makefile:519: ugrep-ugrep.o] Error 1
make[2]: *** Waiting for unfinished jobs....
mv -f .deps/ugrep-output.Tpo .deps/ugrep-output.Po
mv -f .deps/ugrep-query.Tpo .deps/ugrep-query.Po
make[2]: Leaving directory '/tmp/ugrep/src'
make[1]: *** [Makefile:571: all-recursive] Error 1
make[1]: Leaving directory '/tmp/ugrep'
make: *** [Makefile:408: all] Error 2
Failed to build ugrep: please run the following two commands:
$ autoreconf -fi
$ ./build.sh

the repetition ended with the same error

AIX 7.2
gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/opt/freeware/libexec/gcc/powerpc-ibm-aix7.2.0.0/10/lto-wrapper
Target: powerpc-ibm-aix7.2.0.0
Configured with: ../gcc-10.3.0/configure --prefix=/opt/freeware --mandir=/opt/freeware/man --infodir=/opt/freeware/info --with-local-prefix=/opt/freeware --enable-languages=c,c++,fortran,go --enable-version-specific-runtime-libs --disable-nls --disable-libstdcxx-pch --disable-werror --enable-libstdcxx-filesystem-ts --with-gcc-major-version-only --program-suffix=-10 --with-cpu=default32 --host=powerpc-ibm-aix7.2.0.0
Thread model: aix
Supported LTO compression algorithms: zlib
gcc version 10.3.0 (GCC)

@genivia-inc
Copy link
Member

Thanks for the feedback.

Looks like madvise has inconsistent signatures? See e.g. https://man7.org/linux/man-pages/man2/madvise.2.html

And on MacOS also:

int
     madvise(void *addr, size_t len, int advice);

Apparently Solaris and AIX use caddr_t instead of void*.

invalid 'static_cast' from type 'fsid_t' to type 'uint64_t' {aka 'long long unsigned int'}

Will need to find a way to make this portable to Solaris and AIX.

@genivia-inc
Copy link
Member

genivia-inc commented Mar 18, 2024

I need a bit of help here since I don't have AIX to test with.

For the fsid_t problem, I believe the following change should work by defining a function at line 250 in ugrep.cpp to cast instead of explicit casts:

// containers of POSIX-compliant file system ids to exclude from recursive searches or include in recursive searches
std::set<uint64_t> exclude_fs_ids, include_fs_ids;

#if (defined(_AIX) || defined(__TOS_AIX__)) && (defined(__64BIT_KERNEL) || defined(_ALL_SOURCE))
// struct fsid_t with f_fsid.val[2]
inline uint64_t fsid2u64(struct fsid_t& fsid)
{
  return static_cast<uint64_t>(fsid.val[0]) | (static_cast<uint64_t>(fsid.val[1]) << 32);
}
#else
// numeric f_fsid
template<typename T>
inline uint64_t fsid2u64(T& fsid)
{
  return static_cast<uint64_t>(fsid);
}
#endif

Like so in the rest of the code where statvfs() is called:

        // --exclude-fs without MOUNT points and no targets for recursive search: only include the working dir FS
        if (statvfs(".", &buf) == 0)
          include_fs_ids.insert(fsid2u64(buf.f_fsid));

Could you please check for me? I'm not sure if __64BIT_KERNEL and/or _ALL_SOURCE is defined or if we need to define it to force extension compliance.

A similar problem in CPython
python/cpython#76571

References:
https://www.ibm.com/docs/en/aix/7.3?topic=s-statvfs-fstatvfs-statvfs64-fstatvfs64-subroutine

@genivia-inc
Copy link
Member

And for the madvice() in src/mmap.hpp I have this in mind:

#if defined(_AIX)
        madvise(static_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL);
#else
        madvise(mmap_base, mmap_size, MADV_SEQUENTIAL);
#endif

I can't find any AIX man page online to corroborate. The mmap_base pointer is void* on most systems, but apparently char* (caddr_t) on AIX.

Will this work?

Mmap is not used by ugrep by default, unless forced with option --mmap. So it could be disabled if need be.

@genivia-inc genivia-inc changed the title build crashed on AIX build fails on AIX Mar 18, 2024
@genivia-inc genivia-inc added the problem Something isn't working due to a (minor) problem label Mar 18, 2024
@arozehnal
Copy link
Author

I'm sorry, I'm not a programmer. I'm not sure about changes in src/mmap.hpp.
I did this:

#if defined(_AIX)
        madvise(static_cast(mmap_base), mmap_size, MADV_SEQUENTIAL);
#else
        madvise(mmap_base, mmap_size, MADV_SEQUENTIAL);
#endif

// manage mmap state
class MMap {
...

but the problem still persists
In file included from ugrep.cpp:79:
mmap.hpp:61:16: error: expected constructor, destructor, or type conversion before '(' token
61 | madvise(static_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL);
| ^
In file included from ugrep.cpp:79:
mmap.hpp: In member function 'bool MMap::file(reflex::Input&, const char*&, size_t&)':
mmap.hpp:120:17: error: invalid conversion from 'void*' to 'caddr_t' {aka 'char*'} [-fpermissive]
120 | madvise(mmap_base, mmap_size, MADV_SEQUENTIAL);
| ^~~~~~~~~
| |
| void*
In file included from mmap.hpp:52,

@genivia-inc
Copy link
Member

Thanks for trying.

Hm, it might be necessary to do a reinterpret_cast<> like so:

madvise(reinterpret_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL);

It's a bit ugly, but we know for sure we convert pointer types (char* and void*).

@arozehnal
Copy link
Author

I've made some progress. after running:

 ./build.sh --disable-zstd --disable-brotli -disable-lzma -disable-lz4 --disable-zlib --disable-bzip2 --disable-pcre2

I'll get to this point

..
...
mv -f .deps/ugrep-vkey.Tpo .deps/ugrep-vkey.Po
mv -f .deps/ugrep-screen.Tpo .deps/ugrep-screen.Po
mv -f .deps/ugrep-stats.Tpo .deps/ugrep-stats.Po
mv -f .deps/ugrep-cnf.Tpo .deps/ugrep-cnf.Po
mv -f .deps/ugrep-output.Tpo .deps/ugrep-output.Po
mv -f .deps/ugrep-query.Tpo .deps/ugrep-query.Po
mv -f .deps/ugrep-ugrep.Tpo .deps/ugrep-ugrep.Po
g++ -std=gnu++11  -Wall -Wextra -Wunused -O2   -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lpthread ../lib/libreflex.a
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variable10notify_allEv
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variableD1Ev
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variableC1Ev
ld: 0711-317 ERROR: Undefined symbol: _ZTINSt6thread6_StateE
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread6_StateD2Ev
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread15_M_start_threadESt10unique_ptrINS_6_StateESt14default_deleteIS1_EEPFvvE
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt18condition_variable10notify_oneEv
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread4joinEv
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt6thread20hardware_concurrencyEv
ld: 0711-317 ERROR: Undefined symbol: ._ZNSt11this_thread11__sleep_forENSt6chrono8durationIxSt5ratioILx1ELx1EEEENS1_IxS2_ILx1ELx1000000000EEEE
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
collect2: error: ld returned 8 exit status

from discussions on the internet I understood that I have some problem with -lpthread .... I have libraries here /opt/freeware/lib/pthread , but I did not find symbols like ZNSt18condition* in them..
I suspect that they are not implemented for AIX. :-(

@genivia-inc
Copy link
Member

Perhaps the following works as the last step with -lthread instead of -lpthread to execute in the src directory:

g++ -std=gnu++11  -Wall -Wextra -Wunused -O2   -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lthread ../lib/libreflex.a

@arozehnal
Copy link
Author

for the final linking I used -> export CXX="g++ -pthread"

the changes in:
src/mmap.hpp and ugrep.cpp
but I did it differently than you advised, I'll add it later, I've been working on it all night, I'm going to sleep now :-)

@arozehnal
Copy link
Author

I attach diff files
please don't ask me why I changed it this way, I'm not programmere
I tried your recommendations first and when compiling still ended with errors, I ask Github Copilot fix it
compiling was successful, but I don't know if my changes are correct
ugrep is running though, so I'll continue with the compiling of the support libraries and then I'll see

**diff ugrep.cpp ugrep.cpp_ori** 564,571d563 < < uint64_t fsid_to_uint64(fsid_t fsid) { < uint64_t id; < memcpy(&id, &fsid, sizeof(fsid_t)); < return id; < } < < 6859c6851 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid)); --- > include_fs_ids.insert(static_cast(buf.f_fsid)); 6866c6858 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid)); --- > include_fs_ids.insert(static_cast(buf.f_fsid)); 6883c6875 < exclude_fs_ids.insert(fsid_to_uint64(buf.f_fsid)); --- > exclude_fs_ids.insert(static_cast(buf.f_fsid)); 6905c6897 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid)); --- > include_fs_ids.insert(static_cast(buf.f_fsid)); 6921c6913 < include_fs_ids.insert(fsid_to_uint64(buf.f_fsid)); --- > include_fs_ids.insert(static_cast(buf.f_fsid)); 9260c9252 < uint64_t id = fsid_to_uint64(buf.f_fsid); --- > uint64_t id = static_cast(buf.f_fsid); **diff mmap.hpp mmap.hpp_ori** > // --min-mmap and --max-mmap file size to allocate with mmap(), not greater than 4294967295LL, 0 disables mmap() 71a109 > // files are sequentially read 73a112,113 > else > madvise(mmap_base, mmap_size, MADV_SEQUENTIAL); 83,85d122 < { < // files are sequentially read < madvise(reinterpret_cast(mmap_base), mmap_size, MADV_SEQUENTIAL); 87d123 < } 112c148 < #endif --- > #endif

@genivia-inc
Copy link
Member

Thank you for taking the time to work around the AIX build issue.

To make this work in general for portability and safety (because that memcpy() isn't safe as sizeof(fsid_t) is not guaranteed to be 8 bytes):

ugrep/src/ugrep.cpp:255

#if defined(_AIX) || defined(__TOS_AIX__)
// NetBSD compatible struct fsid_t with unsigned integers f_fsid.val[2]
inline uint64_t fsid_to_uint64(fsid_t& fsid)
{
  return static_cast<uint64_t>(fsid.val[0]) | (static_cast<uint64_t>(fsid.val[1]) << 32);
}
#else
// POSIX compliant unsigned integer f_fsid
template<typename T>
inline uint64_t fsid_to_uint64(T& fsid)
{
  return static_cast<uint64_t>(fsid);
}
#endif

ugrep/src/mmap/hpp:113 add reinterpret_cast<caddr_t> like so, which appears to be portable:

madvise(reinterpret_cast<caddr_t>(mmap_base), mmap_size, MADV_SEQUENTIAL)

@genivia-inc genivia-inc added the patch A patch to fix an issue label Mar 22, 2024
@genivia-inc
Copy link
Member

These changes will be included with an upcoming release.

I will close this for now as fixed.

genivia-inc added a commit that referenced this issue Mar 22, 2024
@genivia-inc
Copy link
Member

Fixed in v5.1.2. Disclaimer: I'm not sure if the changes always work to build on AIX. We don't have AIX systems to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A patch to fix an issue problem Something isn't working due to a (minor) problem
Projects
None yet
Development

No branches or pull requests

2 participants