An app developer can sometimes choose how big they want off_t to be (by #defining _FILE_OFFSET_BITS). The default for i686 is 32 bits (a bit odd, that). Pacman's configure attempts to choose 64 bits, and succeeds in doing so for the Arch builds (but doesn't force that choice).
Bad things happen when the app developer chooses differently than configure did, but not until runtime. For example, the app's .o files will think sizeof(alpm_file_t) == 12, whereas libalpm thinks it == 16. The compile and link phases don't catch this.
One possible solution might be to "mv alpm.h alpm.h.in", and add something like:
#if sizeof(off_t) != @???@ #error ??? #endif
Not sure yet what to put in those "???" spots, but the basic idea is to prevent the compiler from generating any .o files that disagree with the ABI of /usr/lib/libalpm. Hope that all made sense, and that I'm not duplicating - I searched the mailing list and the bug database and found zero hits. Anyone see a better solution? Jeremy
On Wed, Nov 20, 2013 at 10:26 PM, Jeremy Heiner scalaprotractor@gmail.com wrote:
#if sizeof(off_t) != @???@
Well, that approach seems like it is not going to work. Obviously cpp can't do sizeof, but I was hoping configure would provide something to fill that gap. However, now I've read through AC_SYS_LARGEFILE and it is pretty clear that there's no help there.
Changing alpm.h to use off64_t would be an obstacle to running on platforms like smartphones, so I assume that is out. There doesn't seem to be a portable way to keep the compiler from generating .o files that differ from the libalpm ABI. Nor can the linker detect those differences (maybe it could be made to, but not without significant changes to the API).
Leaving only one semi-solution that I can see at this moment: document this behavior. There's gotta be something I've missed?
On 22/11/13 01:19, Jeremy Heiner wrote:
On Wed, Nov 20, 2013 at 10:26 PM, Jeremy Heiner scalaprotractor@gmail.com wrote:
#if sizeof(off_t) != @???@
Well, that approach seems like it is not going to work. Obviously cpp can't do sizeof, but I was hoping configure would provide something to fill that gap. However, now I've read through AC_SYS_LARGEFILE and it is pretty clear that there's no help there.
Changing alpm.h to use off64_t would be an obstacle to running on platforms like smartphones, so I assume that is out. There doesn't seem to be a portable way to keep the compiler from generating .o files that differ from the libalpm ABI. Nor can the linker detect those differences (maybe it could be made to, but not without significant changes to the API).
Leaving only one semi-solution that I can see at this moment: document this behavior. There's gotta be something I've missed?
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
On Thu, Nov 21, 2013 at 3:52 PM, Allan McRae allan@archlinux.org wrote:
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
How about the approach outlined in the attachment? It passes 'make check' and performs correctly when a libalpm client app goofs up.
On 22/11/13 22:37, Jeremy Heiner wrote:
On Thu, Nov 21, 2013 at 3:52 PM, Allan McRae allan@archlinux.org wrote:
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
How about the approach outlined in the attachment? It passes 'make check' and performs correctly when a libalpm client app goofs up.
Please send patches inline so we can easily comment.
I was quite surprised this worked:
+#define alpm_initialize(root,dbpath,err) \ + alpm_initialize_check_largefile(root, dbpath, err, sizeof(off_t))
The sizeof(off_t) gets evaluated before the #define? That can't be right... So must be optimised out? I'm either missing something here or this is prone to breakage.
I guess a working approach would be to find the size of off_t during configure (e.g. [1] - there are other approaches) and test sizeof(off_t) == SIZEOF_OFF_T in alpm_initialise.
Allan
[1] http://www.gnu.org/software/autoconf-archive/ax_compile_check_sizeof.html
On 11/25/13 at 09:08pm, Allan McRae wrote:
On 22/11/13 22:37, Jeremy Heiner wrote:
On Thu, Nov 21, 2013 at 3:52 PM, Allan McRae allan@archlinux.org wrote:
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
How about the approach outlined in the attachment? It passes 'make check' and performs correctly when a libalpm client app goofs up.
Please send patches inline so we can easily comment.
I was quite surprised this worked:
+#define alpm_initialize(root,dbpath,err) \
- alpm_initialize_check_largefile(root, dbpath, err, sizeof(off_t))
The sizeof(off_t) gets evaluated before the #define? That can't be right... So must be optimised out? I'm either missing something here or this is prone to breakage.
If I'm not mistaken, sizeof is evaluated at compile time. This works because the sizeof(off_t) in the #define is evaluated when the calling program is compiled and the sizeof(off_t) inside alpm_initialize_check_largefile is evaluated when alpm is compiled.
I guess a working approach would be to find the size of off_t during configure (e.g. [1] - there are other approaches) and test sizeof(off_t) == SIZEOF_OFF_T in alpm_initialise.
I don't see how this would work. Because sizeof is a compile-time operation, it *must* be provided by the program using alpm.
Allan
[1] http://www.gnu.org/software/autoconf-archive/ax_compile_check_sizeof.html
On 25/11/13 23:43, Andrew Gregory wrote:
On 11/25/13 at 09:08pm, Allan McRae wrote:
On 22/11/13 22:37, Jeremy Heiner wrote:
On Thu, Nov 21, 2013 at 3:52 PM, Allan McRae allan@archlinux.org wrote:
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
How about the approach outlined in the attachment? It passes 'make check' and performs correctly when a libalpm client app goofs up.
Please send patches inline so we can easily comment.
I was quite surprised this worked:
+#define alpm_initialize(root,dbpath,err) \
- alpm_initialize_check_largefile(root, dbpath, err, sizeof(off_t))
The sizeof(off_t) gets evaluated before the #define? That can't be right... So must be optimised out? I'm either missing something here or this is prone to breakage.
If I'm not mistaken, sizeof is evaluated at compile time. This works because the sizeof(off_t) in the #define is evaluated when the calling program is compiled and the sizeof(off_t) inside alpm_initialize_check_largefile is evaluated when alpm is compiled.
Ah - compile time evaluation... I understand what this is doing now. However, I don't like that this is a runtime check.
From searching around, we are definitely not the first people to run
into this issue, but seem to be the only ones who are trying to code around.
Allan
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
On 25/11/13 23:43, Andrew Gregory wrote:
On 11/25/13 at 09:08pm, Allan McRae wrote:
On 22/11/13 22:37, Jeremy Heiner wrote:
On Thu, Nov 21, 2013 at 3:52 PM, Allan McRae allan@archlinux.org wrote:
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
How about the approach outlined in the attachment? It passes 'make check' and performs correctly when a libalpm client app goofs up.
Please send patches inline so we can easily comment.
I was quite surprised this worked:
+#define alpm_initialize(root,dbpath,err) \
- alpm_initialize_check_largefile(root, dbpath, err, sizeof(off_t))
The sizeof(off_t) gets evaluated before the #define? That can't be right... So must be optimised out? I'm either missing something here or this is prone to breakage.
If I'm not mistaken, sizeof is evaluated at compile time. This works because the sizeof(off_t) in the #define is evaluated when the calling program is compiled and the sizeof(off_t) inside alpm_initialize_check_largefile is evaluated when alpm is compiled.
Ah - compile time evaluation... I understand what this is doing now. However, I don't like that this is a runtime check.
From searching around, we are definitely not the first people to run into this issue, but seem to be the only ones who are trying to code around.
Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
On Mon, Nov 25, 2013 at 09:30:01AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
On 25/11/13 23:43, Andrew Gregory wrote:
On 11/25/13 at 09:08pm, Allan McRae wrote:
On 22/11/13 22:37, Jeremy Heiner wrote:
On Thu, Nov 21, 2013 at 3:52 PM, Allan McRae allan@archlinux.org wrote:
Nope - documenting is the only real way to deal with this. For example:
http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_...
How about the approach outlined in the attachment? It passes 'make check' and performs correctly when a libalpm client app goofs up.
Please send patches inline so we can easily comment.
I was quite surprised this worked:
+#define alpm_initialize(root,dbpath,err) \
- alpm_initialize_check_largefile(root, dbpath, err, sizeof(off_t))
The sizeof(off_t) gets evaluated before the #define? That can't be right... So must be optimised out? I'm either missing something here or this is prone to breakage.
If I'm not mistaken, sizeof is evaluated at compile time. This works because the sizeof(off_t) in the #define is evaluated when the calling program is compiled and the sizeof(off_t) inside alpm_initialize_check_largefile is evaluated when alpm is compiled.
Ah - compile time evaluation... I understand what this is doing now. However, I don't like that this is a runtime check.
From searching around, we are definitely not the first people to run into this issue, but seem to be the only ones who are trying to code around.
Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
A cursory examination of alpm.h shows that this is definitely not the case -- it isn't only the "file stuff", and even that API isn't abstracted away since we publicly expose off_t in some of our structs. And, we have callbacks which pass off_t as well. At best, you read a corrupt value. At worst, you crash after wrongly calculating offsets of other struct members.
This isn't a new problem for alpm or anyone else dealing with large files. It's simply a trait of the target system which has to be adhered to.
I propose we do nothing.
d
On Mon, Nov 25, 2013 at 10:27 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 09:30:01AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
From searching around, we are definitely not the first people to run into this issue, but seem to be the only ones who are trying to code around.
Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
A cursory examination of alpm.h shows that this is definitely not the case -- it isn't only the "file stuff", and even that API isn't abstracted away since we publicly expose off_t in some of our structs. And, we have callbacks which pass off_t as well. At best, you read a corrupt value. At worst, you crash after wrongly calculating offsets of other struct members.
This isn't a new problem for alpm or anyone else dealing with large files. It's simply a trait of the target system which has to be adhered to.
I propose we do nothing.
d
Hi, Dave. Thanks for your reply, but I am a bit confused. I said that the only exposure the libalpm API has to the largefile problems is that it passes off_t across. You seem to be saying there is greater exposure than that, but all the examples you pointed to are where off_t is being passed across the API. I did look carefully through the API for any other largefile issues and found zero. Of course I am only human. So if, as you said, that "is definitely not the case", then could you please point me to where the exposure (other than off_t) is in the libalpm API? Jeremy
On Mon, Nov 25, 2013 at 10:54:05AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 10:27 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 09:30:01AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
From searching around, we are definitely not the first people to run into this issue, but seem to be the only ones who are trying to code around.
Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
A cursory examination of alpm.h shows that this is definitely not the case -- it isn't only the "file stuff", and even that API isn't abstracted away since we publicly expose off_t in some of our structs. And, we have callbacks which pass off_t as well. At best, you read a corrupt value. At worst, you crash after wrongly calculating offsets of other struct members.
This isn't a new problem for alpm or anyone else dealing with large files. It's simply a trait of the target system which has to be adhered to.
I propose we do nothing.
d
Hi, Dave. Thanks for your reply, but I am a bit confused. I said that the only exposure the libalpm API has to the largefile problems is that it passes off_t across. You seem to be saying there is greater exposure than that, but all the examples you pointed to are where off_t is being passed across the API. I did look carefully through the API for any other largefile issues and found zero. Of course I am only human. So if, as you said, that "is definitely not the case", then could you please point me to where the exposure (other than off_t) is in the libalpm API? Jeremy
Your confusion confuses me, since you seem to point out the problem and then dismiss it for reasons unknown. _FILE_OFFSET_BITS is explicitly for determining the size of off_t and the usage of various *64 functions. I'm not sure why you're expecting other cases that aren't directly related to the usage of off_t and passing it across API calls.
If the library and the application have different definitions of how large an off_t is, then you run into potential corruption and crashes.
Consider the following struct:
struct alpm_file_t { char *name; off_t size; mode_t mode; }
When compiling for i686, this struct changes its size (and therefore changes ABI) based on the value of _FILE_OFFSET_BITS:
32: the struct is 12 bytes 64: the struct is 16 bytes
This isn't relevant for x86_64 where off_t is always 64 bits.
If an i686 system compiles libalpm without large file support and then compiles an application *with* large file support that links to alpm, you read a full 64 bits when encountering an off_t (which is wrong). The inverse of this (where ALPM has LFS and the application does not) is equally painful as it can still result in miscalculation of the offsets of struct members.
d
On Mon, Nov 25, 2013 at 11:20 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 10:54:05AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 10:27 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 09:30:01AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
From searching around, we are definitely not the first people to run into this issue, but seem to be the only ones who are trying to code around.
Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
A cursory examination of alpm.h shows that this is definitely not the case -- it isn't only the "file stuff", and even that API isn't abstracted away since we publicly expose off_t in some of our structs. And, we have callbacks which pass off_t as well. At best, you read a corrupt value. At worst, you crash after wrongly calculating offsets of other struct members.
This isn't a new problem for alpm or anyone else dealing with large files. It's simply a trait of the target system which has to be adhered to.
I propose we do nothing.
d
Hi, Dave. Thanks for your reply, but I am a bit confused. I said that the only exposure the libalpm API has to the largefile problems is that it passes off_t across. You seem to be saying there is greater exposure than that, but all the examples you pointed to are where off_t is being passed across the API. I did look carefully through the API for any other largefile issues and found zero. Of course I am only human. So if, as you said, that "is definitely not the case", then could you please point me to where the exposure (other than off_t) is in the libalpm API? Jeremy
Your confusion confuses me, since you seem to point out the problem and then dismiss it for reasons unknown. _FILE_OFFSET_BITS is explicitly for determining the size of off_t and the usage of various *64 functions. I'm not sure why you're expecting other cases that aren't directly related to the usage of off_t and passing it across API calls.
If the library and the application have different definitions of how large an off_t is, then you run into potential corruption and crashes.
Consider the following struct:
struct alpm_file_t { char *name; off_t size; mode_t mode; }
When compiling for i686, this struct changes its size (and therefore changes ABI) based on the value of _FILE_OFFSET_BITS:
32: the struct is 12 bytes 64: the struct is 16 bytes
This isn't relevant for x86_64 where off_t is always 64 bits.
If an i686 system compiles libalpm without large file support and then compiles an application *with* large file support that links to alpm, you read a full 64 bits when encountering an off_t (which is wrong). The inverse of this (where ALPM has LFS and the application does not) is equally painful as it can still result in miscalculation of the offsets of struct members.
d
Yes, what you have described is the extent of the exposure to the problem that the libalpm API has. Allen was pointing to other libraries which have greater exposure (e.g. the file descriptors passed across the GPGME API) and suggesting that their solution be adopted universally. My point was that the two different levels of exposure are different. The lower level admits a solution, while the greater level seems intractable. The solution I proposed addresses the exposure that libalpm has, but would be inadequate for GPGME. You seem to be suggesting that my proposal for libalpm be rejected because it fails to solve GPGME's problem. That is the source of my confusion. Jeremy
On Mon, Nov 25, 2013 at 11:56:54AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 11:20 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 10:54:05AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 10:27 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 09:30:01AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
From searching around, we are definitely not the first people to run into this issue, but seem to be the only ones who are trying to code around.
Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
A cursory examination of alpm.h shows that this is definitely not the case -- it isn't only the "file stuff", and even that API isn't abstracted away since we publicly expose off_t in some of our structs. And, we have callbacks which pass off_t as well. At best, you read a corrupt value. At worst, you crash after wrongly calculating offsets of other struct members.
This isn't a new problem for alpm or anyone else dealing with large files. It's simply a trait of the target system which has to be adhered to.
I propose we do nothing.
d
Hi, Dave. Thanks for your reply, but I am a bit confused. I said that the only exposure the libalpm API has to the largefile problems is that it passes off_t across. You seem to be saying there is greater exposure than that, but all the examples you pointed to are where off_t is being passed across the API. I did look carefully through the API for any other largefile issues and found zero. Of course I am only human. So if, as you said, that "is definitely not the case", then could you please point me to where the exposure (other than off_t) is in the libalpm API? Jeremy
Your confusion confuses me, since you seem to point out the problem and then dismiss it for reasons unknown. _FILE_OFFSET_BITS is explicitly for determining the size of off_t and the usage of various *64 functions. I'm not sure why you're expecting other cases that aren't directly related to the usage of off_t and passing it across API calls.
If the library and the application have different definitions of how large an off_t is, then you run into potential corruption and crashes.
Consider the following struct:
struct alpm_file_t { char *name; off_t size; mode_t mode; }
When compiling for i686, this struct changes its size (and therefore changes ABI) based on the value of _FILE_OFFSET_BITS:
32: the struct is 12 bytes 64: the struct is 16 bytes
This isn't relevant for x86_64 where off_t is always 64 bits.
If an i686 system compiles libalpm without large file support and then compiles an application *with* large file support that links to alpm, you read a full 64 bits when encountering an off_t (which is wrong). The inverse of this (where ALPM has LFS and the application does not) is equally painful as it can still result in miscalculation of the offsets of struct members.
d
Yes, what you have described is the extent of the exposure to the problem that the libalpm API has. Allen was pointing to other libraries which have greater exposure (e.g. the file descriptors passed across the GPGME API) and suggesting that their solution be adopted universally. My point was that the two different levels of exposure are different. The lower level admits a solution, while the greater level seems intractable. The solution I proposed addresses the exposure that libalpm has, but would be inadequate for GPGME. You seem to be suggesting that my proposal for libalpm be rejected because it fails to solve GPGME's problem. That is the source of my confusion. Jeremy
I think Allan was simply pointing out an instance where a library documents the effect of LFS on the library. The extent of the exposure doesn't seem worth quantifying. It's an ABI mismatch to compile some things with LFS and some without. IMO, this is a target machine concern, not one of every single library to perform due dilligence. Do you also want to try and code around various compiler flags which can affect ABI (see gcc(1) for flags like -mstack-offset, or -m{soft,hard}-float)?
If you'd still prefer to solve this, I'd suggest you take a look at /usr/include/curl/curl{build-*,rules}.h which solve similar mismatches at compile time.
I'll also mention that rtorrent used to have a runtime check for this to ensure that sizeof(off_t) == 8, but this was mostly because the software hadn't been thoroughly tested with sizeof(off_t) == 4. So, do we know that ALPM behaves sanely when LFS isn't enabled?
Really think this should just be solved by documenting...
d
On Mon, Nov 25, 2013 at 1:28 PM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 11:56:54AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 11:20 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 10:54:05AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 10:27 AM, Dave Reisner d@falconindy.com wrote:
On Mon, Nov 25, 2013 at 09:30:01AM -0500, Jeremy Heiner wrote:
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote: > From searching around, we are definitely not the first people to run > into this issue, but seem to be the only ones who are trying to code around. > > Allan
I think the reason is that most libraries that face this issue have more exposure to it than libalpm does. For example GPGME passes file descriptors across its API, and that is a much bigger can of worms. It's not surprising that they choose to avoid that mess. But the libalpm API only passes off_t across. All the file stuff is completely encapsulated, so most of the largefile problems are simply not problems for libalpm. Jeremy
A cursory examination of alpm.h shows that this is definitely not the case -- it isn't only the "file stuff", and even that API isn't abstracted away since we publicly expose off_t in some of our structs. And, we have callbacks which pass off_t as well. At best, you read a corrupt value. At worst, you crash after wrongly calculating offsets of other struct members.
This isn't a new problem for alpm or anyone else dealing with large files. It's simply a trait of the target system which has to be adhered to.
I propose we do nothing.
d
Hi, Dave. Thanks for your reply, but I am a bit confused. I said that the only exposure the libalpm API has to the largefile problems is that it passes off_t across. You seem to be saying there is greater exposure than that, but all the examples you pointed to are where off_t is being passed across the API. I did look carefully through the API for any other largefile issues and found zero. Of course I am only human. So if, as you said, that "is definitely not the case", then could you please point me to where the exposure (other than off_t) is in the libalpm API? Jeremy
Your confusion confuses me, since you seem to point out the problem and then dismiss it for reasons unknown. _FILE_OFFSET_BITS is explicitly for determining the size of off_t and the usage of various *64 functions. I'm not sure why you're expecting other cases that aren't directly related to the usage of off_t and passing it across API calls.
If the library and the application have different definitions of how large an off_t is, then you run into potential corruption and crashes.
Consider the following struct:
struct alpm_file_t { char *name; off_t size; mode_t mode; }
When compiling for i686, this struct changes its size (and therefore changes ABI) based on the value of _FILE_OFFSET_BITS:
32: the struct is 12 bytes 64: the struct is 16 bytes
This isn't relevant for x86_64 where off_t is always 64 bits.
If an i686 system compiles libalpm without large file support and then compiles an application *with* large file support that links to alpm, you read a full 64 bits when encountering an off_t (which is wrong). The inverse of this (where ALPM has LFS and the application does not) is equally painful as it can still result in miscalculation of the offsets of struct members.
d
Yes, what you have described is the extent of the exposure to the problem that the libalpm API has. Allen was pointing to other libraries which have greater exposure (e.g. the file descriptors passed across the GPGME API) and suggesting that their solution be adopted universally. My point was that the two different levels of exposure are different. The lower level admits a solution, while the greater level seems intractable. The solution I proposed addresses the exposure that libalpm has, but would be inadequate for GPGME. You seem to be suggesting that my proposal for libalpm be rejected because it fails to solve GPGME's problem. That is the source of my confusion. Jeremy
I think Allan was simply pointing out an instance where a library documents the effect of LFS on the library. The extent of the exposure doesn't seem worth quantifying. It's an ABI mismatch to compile some things with LFS and some without. IMO, this is a target machine concern, not one of every single library to perform due dilligence. Do you also want to try and code around various compiler flags which can affect ABI (see gcc(1) for flags like -mstack-offset, or -m{soft,hard}-float)?
If you'd still prefer to solve this, I'd suggest you take a look at /usr/include/curl/curl{build-*,rules}.h which solve similar mismatches at compile time.
I'll also mention that rtorrent used to have a runtime check for this to ensure that sizeof(off_t) == 8, but this was mostly because the software hadn't been thoroughly tested with sizeof(off_t) == 4. So, do we know that ALPM behaves sanely when LFS isn't enabled?
Really think this should just be solved by documenting...
d
Yes, Allan cited GPGME's "just document it" approach as typical. Which it is. I'm only pointing out that libalpm has a superior API when it comes to largefile issues, and thus has options that are not available to those more typical libraries. How is it not worth distinguishing between having some options available versus having none?
I agree with you that libraries should _not_ bear the responsibility to check every single potential ABI mismatch. But it is not an all-or-nothing situation. There is no slippery slope from doing simple checks for common stumbling blocks to checking every single potential ABI mismatch. The impossibility of checking absolutely everything is not a good reason to omit checks that are easy and appropriate.
The curl approach is clearly overkill for the libalpm problem. Again you seem to be criticizing my proposal for libalpm because it fails to solve some other library's problem. And that still confuses me.
The behavior of libalpm without LFS is an interesting question, but not at all relevant to what I've proposed. I'm fairly certain that the committee that gave us LFS seventeen(?) years ago would say "it'll work fine". Then again, they did give us LFS. ;) Jeremy
On 26/11/13 17:14, Jeremy Heiner wrote: <snip>
The curl approach is clearly overkill for the libalpm problem. Again you seem to be criticizing my proposal for libalpm because it fails to solve some other library's problem. And that still confuses me.
Can someone summarise - or link to code - for how libcurl handles this? Even if it is overkill, I am still looking for something cleaner than the proposed #define/#undef approach which feels a bit hacky (even if it works).
Allan
On Tue, Dec 10, 2013 at 11:58 PM, Allan McRae allan@archlinux.org wrote:
I am still looking for something cleaner than the proposed #define/#undef approach which feels a bit hacky
Here is a third (well, 4th, since "do nothing" is always on the table) option which lies between "just document" and "macro shenanigans"... Following the lead of 'alpm_capabilities', add:
/** with documentation of the issue */ size_t alpm_sizeof_off_t(void);
It's not automatic like the macro approach, so it is one more thing the app writer has to at least consider. But it makes the needed information available (unlike the "just document" approach, where all you can do is run the app to see if it SEGVs).
I'd be happy to submit a patch implementing this approach if it passes muster. Jeremy
On 12/12/13 00:24, Jeremy Heiner wrote:
On Tue, Dec 10, 2013 at 11:58 PM, Allan McRae allan@archlinux.org wrote:
I am still looking for something cleaner than the proposed #define/#undef approach which feels a bit hacky
Here is a third (well, 4th, since "do nothing" is always on the table) option which lies between "just document" and "macro shenanigans"... Following the lead of 'alpm_capabilities', add:
How about #5...
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 878c38b..a41c07d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -52,6 +52,12 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, const char *lf = "db.lck"; size_t lockfilelen; alpm_handle_t *myhandle = _alpm_handle_new(); + + /* calculate off_t size at runtime */ + size_t off_t_size = ((char *)((off_t *)0 + 1) - (char *)(off_t *)0); + + if(off_t_size != sizeof(off_t)) { + myerr = ALPM_ERR_OFF_T_SIZE; + goto cleanup; + }
if(myhandle == NULL) { myerr = ALPM_ERR_MEMORY; diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5fa775c..f61e3f5 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1333,6 +1333,7 @@ typedef enum _alpm_errno_t { /* Misc */ ALPM_ERR_RETRIEVE, ALPM_ERR_INVALID_REGEX, + ALPM_ERR_OFF_T_SIZE, /* External library errors */ ALPM_ERR_LIBARCHIVE, ALPM_ERR_LIBCURL, diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 8622180..1aed1a1 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -145,6 +145,8 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err) return _("failed to retrieve some files"); case ALPM_ERR_INVALID_REGEX: return _("invalid regular expression"); + case ALPM_ERR_OFF_T_SIZE: + return _("incompatible off_t size"); /* Errors from external libraries- our own wrapper error */ case ALPM_ERR_LIBARCHIVE: /* it would be nice to use archive_error_string() here, but that
On Wed, Dec 11, 2013 at 11:14 AM, Allan McRae allan@archlinux.org wrote:
How about #5...
Remember: libalpm is a .so, and sizeof(off_t) is something the app writer is free to choose (on many systems). The problem is that the choice made by libalpm (by ./configure) gets baked into the .so ABI, and if the app writer chooses differently they get SEGVs.
The macro approach works because the macro gets evaluated in the app compile context, so it sees what the app writer chose. Without knowing both what libalpm ./configure chose and what the app writer chose you can't detect when they differ.
On 12/12/13 02:42, Jeremy Heiner wrote:
On Wed, Dec 11, 2013 at 11:14 AM, Allan McRae allan@archlinux.org wrote:
How about #5...
Remember: libalpm is a .so, and sizeof(off_t) is something the app writer is free to choose (on many systems). The problem is that the choice made by libalpm (by ./configure) gets baked into the .so ABI, and if the app writer chooses differently they get SEGVs.
Yes. That is why I was proposing an approach that had sizeof(off_t) baked in and a second calculation done at runtime. It turns out my "runtime" calculation was easily optimized out... Fixed version on the way.
A
On Thu, Dec 12, 2013 at 02:14:19AM +1000, Allan McRae wrote:
On 12/12/13 00:24, Jeremy Heiner wrote:
On Tue, Dec 10, 2013 at 11:58 PM, Allan McRae allan@archlinux.org wrote:
I am still looking for something cleaner than the proposed #define/#undef approach which feels a bit hacky
Here is a third (well, 4th, since "do nothing" is always on the table) option which lies between "just document" and "macro shenanigans"... Following the lead of 'alpm_capabilities', add:
How about #5...
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 878c38b..a41c07d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -52,6 +52,12 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, const char *lf = "db.lck"; size_t lockfilelen; alpm_handle_t *myhandle = _alpm_handle_new();
- /* calculate off_t size at runtime */
- size_t off_t_size = ((char *)((off_t *)0 + 1) - (char *)(off_t *)0);
- if(off_t_size != sizeof(off_t)) {
myerr = ALPM_ERR_OFF_T_SIZE;goto cleanup;- }
I do not believe this actually works as intended. There's nothing "at runtime" about this, especially when optimizations are involved (even with -O1). Your off_t_size will be calculated at compile time and therefore the comparison that follows will never fail. The instructions generated for the off_t_size is effectively:
mov $0x8,%eax
Or, when LFS isn't enabled:
mov $0x4,%eax
d
if(myhandle == NULL) { myerr = ALPM_ERR_MEMORY; diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5fa775c..f61e3f5 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1333,6 +1333,7 @@ typedef enum _alpm_errno_t { /* Misc */ ALPM_ERR_RETRIEVE, ALPM_ERR_INVALID_REGEX,
- ALPM_ERR_OFF_T_SIZE, /* External library errors */ ALPM_ERR_LIBARCHIVE, ALPM_ERR_LIBCURL,
diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 8622180..1aed1a1 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -145,6 +145,8 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err) return _("failed to retrieve some files"); case ALPM_ERR_INVALID_REGEX: return _("invalid regular expression");
case ALPM_ERR_OFF_T_SIZE:return _("incompatible off_t size");
On 12/12/13 02:44, Dave Reisner wrote:
On Thu, Dec 12, 2013 at 02:14:19AM +1000, Allan McRae wrote:
On 12/12/13 00:24, Jeremy Heiner wrote:
On Tue, Dec 10, 2013 at 11:58 PM, Allan McRae allan@archlinux.org wrote:
I am still looking for something cleaner than the proposed #define/#undef approach which feels a bit hacky
Here is a third (well, 4th, since "do nothing" is always on the table) option which lies between "just document" and "macro shenanigans"... Following the lead of 'alpm_capabilities', add:
How about #5...
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 878c38b..a41c07d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -52,6 +52,12 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, const char *lf = "db.lck"; size_t lockfilelen; alpm_handle_t *myhandle = _alpm_handle_new();
- /* calculate off_t size at runtime */
- size_t off_t_size = ((char *)((off_t *)0 + 1) - (char *)(off_t *)0);
- if(off_t_size != sizeof(off_t)) {
myerr = ALPM_ERR_OFF_T_SIZE;goto cleanup;- }
I do not believe this actually works as intended. There's nothing "at runtime" about this, especially when optimizations are involved (even with -O1). Your off_t_size will be calculated at compile time and therefore the comparison that follows will never fail. The instructions generated for the off_t_size is effectively:
mov $0x8,%eaxOr, when LFS isn't enabled:
mov $0x4,%eax
This version looks like it should survive optimization:
off_t x; size_t off_t_size = ((char *)(&(x) + 1) - (char *)&(x));
if(myhandle == NULL) { myerr = ALPM_ERR_MEMORY; diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5fa775c..f61e3f5 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1333,6 +1333,7 @@ typedef enum _alpm_errno_t { /* Misc */ ALPM_ERR_RETRIEVE, ALPM_ERR_INVALID_REGEX,
- ALPM_ERR_OFF_T_SIZE, /* External library errors */ ALPM_ERR_LIBARCHIVE, ALPM_ERR_LIBCURL,
diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 8622180..1aed1a1 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -145,6 +145,8 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err) return _("failed to retrieve some files"); case ALPM_ERR_INVALID_REGEX: return _("invalid regular expression");
case ALPM_ERR_OFF_T_SIZE:return _("incompatible off_t size");
On 12/12/13 03:02, Allan McRae wrote:
On 12/12/13 02:44, Dave Reisner wrote:
On Thu, Dec 12, 2013 at 02:14:19AM +1000, Allan McRae wrote:
On 12/12/13 00:24, Jeremy Heiner wrote:
On Tue, Dec 10, 2013 at 11:58 PM, Allan McRae allan@archlinux.org wrote:
I am still looking for something cleaner than the proposed #define/#undef approach which feels a bit hacky
Here is a third (well, 4th, since "do nothing" is always on the table) option which lies between "just document" and "macro shenanigans"... Following the lead of 'alpm_capabilities', add:
How about #5...
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 878c38b..a41c07d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -52,6 +52,12 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, const char *lf = "db.lck"; size_t lockfilelen; alpm_handle_t *myhandle = _alpm_handle_new();
- /* calculate off_t size at runtime */
- size_t off_t_size = ((char *)((off_t *)0 + 1) - (char *)(off_t *)0);
- if(off_t_size != sizeof(off_t)) {
myerr = ALPM_ERR_OFF_T_SIZE;goto cleanup;- }
I do not believe this actually works as intended. There's nothing "at runtime" about this, especially when optimizations are involved (even with -O1). Your off_t_size will be calculated at compile time and therefore the comparison that follows will never fail. The instructions generated for the off_t_size is effectively:
mov $0x8,%eaxOr, when LFS isn't enabled:
mov $0x4,%eaxThis version looks like it should survive optimization:
off_t x; size_t off_t_size = ((char *)(&(x) + 1) - (char *)&(x));
And nope... smart compiler.
On Mon, Nov 25, 2013 at 9:12 AM, Allan McRae allan@archlinux.org wrote:
However, I don't like that this is a runtime check.
I think it has to be, since libalpm is a .so
pacman-dev@lists.archlinux.org